-
Notifications
You must be signed in to change notification settings - Fork 269
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 pre-commit hooks, black formatting, and style fixes #1341
Add pre-commit hooks, black formatting, and style fixes #1341
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
These are the remaining code styling issues:
Hoping to finish this up this week. |
2e42f93
to
cce5fb7
Compare
Home stretch... these are the remaining updates required
|
@mgrover1 Good work on this! I'll take a look, might take a bit with 230 files haha, but I'll get a review going |
Great - thanks! Yeah... no worries about it taking a bit |
- pip: | ||
- wheel | ||
- watchdog | ||
- flake8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want all of these in our user environment file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, if people are going to be developing with this, I think they should be? They are relatively straight forward packages to install (and well supported)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mgrover1 I would say maybe a base environment.yml for users and an environment_dev.yml, for those who want more.
coverage, twine, sphinx etc I doubt a user will use, maybe pytest.
Alrighty thanks for the comments so far @zssherman - I made those suggested changes! |
- pip: | ||
- wheel | ||
- watchdog | ||
- flake8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mgrover1 I would say maybe a base environment.yml for users and an environment_dev.yml, for those who want more.
coverage, twine, sphinx etc I doubt a user will use, maybe pytest.
@zssherman I think its worth keeping it more simple, with everything in the The packages included here are pretty well maintained |
@mgrover1 Sounds good, reverted environment files. |
This looks good to me! |
@zssherman should we go ahead and merge? |
@mgrover1 Yeah I think this is good to go! |
This make sure our code is style-compliant, and adds tools to help with this.