Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add black to requirements and pre-commit #99

Merged
merged 2 commits into from
Oct 20, 2023
Merged

Conversation

caneff
Copy link
Collaborator

@caneff caneff commented Oct 19, 2023

No description provided.

@caneff caneff force-pushed the black branch 2 times, most recently from d450db9 to 0441e49 Compare October 19, 2023 10:41
@caneff caneff force-pushed the black branch 3 times, most recently from 1e75d2f to 773a73c Compare October 19, 2023 10:55
@caneff
Copy link
Collaborator Author

caneff commented Oct 19, 2023

OK I think I addressed all your points from the other PR. Sorry it was easier for me to just close that one and continue here.

@caneff caneff force-pushed the black branch 4 times, most recently from 72c635f to 6597b2c Compare October 19, 2023 11:11
@caneff
Copy link
Collaborator Author

caneff commented Oct 19, 2023

OK fixed the remaining issues

Copy link
Owner

@nanne-aben nanne-aben left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry to be nitpicking, but I feel like the code in the notebooks is sometimes formatted a bit funky. I've made some suggestions to add a , in some places. If I'm not mistaken, black will use these to reformat these pieces of codes with some newlines, making them more readable. I've also made suggestions to remove some redundant ().

docs/source/advanced.ipynb Outdated Show resolved Hide resolved
docs/source/advanced.ipynb Outdated Show resolved Hide resolved
docs/source/deepdive_into_dtypes.ipynb Outdated Show resolved Hide resolved
docs/source/deepdive_into_dtypes.ipynb Outdated Show resolved Hide resolved
docs/source/deepdive_into_dtypes.ipynb Outdated Show resolved Hide resolved
docs/source/getting_started.ipynb Outdated Show resolved Hide resolved
docs/source/getting_started.ipynb Outdated Show resolved Hide resolved
@caneff
Copy link
Collaborator Author

caneff commented Oct 19, 2023

OK think I did them all.

@caneff caneff requested a review from nanne-aben October 19, 2023 15:20
@caneff caneff force-pushed the black branch 3 times, most recently from ab9f2e0 to 2a8426c Compare October 19, 2023 15:26
Follow on PR to reformat existing files
Copy link
Owner

@nanne-aben nanne-aben left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks for your contribution!

I pushed one commit myself, wanted to make some more small edits in the formatting of the notebooks and didn't want to bother you with that :)

@nanne-aben nanne-aben merged commit 4dcb86c into nanne-aben:main Oct 20, 2023
5 checks passed
@caneff caneff mentioned this pull request Oct 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants