Minor style updates and QOL changes #830
Conversation
updated whitespacing and performed minor style changes to conform to flake8 standards.
Local Workspace settings do not need to be shared with everyone.
with flake8-docstrings and the docstring-convention=numpy flake8 will detect style issues
Allows anyone to create a dev conda environment with 'conda env create' from root directory. This makes getting started easier.
Codecov Report
@@ Coverage Diff @@
## master #830 +/- ##
==========================================
+ Coverage 90.32% 90.33% +<.01%
==========================================
Files 96 96
Lines 12192 12193 +1
Branches 2136 2136
==========================================
+ Hits 11013 11014 +1
Misses 834 834
Partials 345 345
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Thanks! We haven't really concerned ourselves with the styles outside the package source, but it's reasonable to conform them. The history of the flake8 styles is that we used a small number of exceptions, and then they changed the defaults. IIRC we just added the exceptions to restore the old behavior, but if you have an interest in looking through them all and justifying or dropping them, that would be fine with me.
I don't think we'll update to black just now. While on the whole I like their aims, there are cases where I think readability suffers. The recoders in your nifti1.py file are good examples of that.
The potential issue with adding things like environment.yml is that we now have to keep it in sync with the other places that we specify dependencies. It would be ideal if we could specify dependencies in setup.cfg and update all of the environment specifications via a script. Would you be able to generate environment.yml from a script like tools/update_requirements.py?
I made some other comments, as well.
Also, sorry about the delay. I wanted to get the release candidate out before digging into this.
| @@ -103,18 +108,14 @@ def rotate_box(box_def, angle, origin): | |||
| box_def_zeroed = box_def - origin | |||
| cost = math.cos(angle) | |||
| sint = math.sin(angle) | |||
| rot_array = np.array([[cost, -sint], | |||
| [sint, cost]]) | |||
| rot_array = np.array([[cost, -sint], [sint, cost]]) | |||
| pt[1] - markersize / 2, | ||
| text, | ||
| color=color) | ||
| plt.text(pt[0] + markersize / 2, pt[1] - markersize / 2, text, color=color) |
| 'opt suffix' : '; you may get run-time errors', | ||
| 'version too old': 'You have version %s of package "%s"' | ||
| ' but we need version >= %s', } | ||
| 'missing': 'Cannot import package "%s" - is it installed?', |
| log.info("No #!python executable found, skipping .bat " | ||
| "wrapper") | ||
| if not (first_line.startswith('#!') and 'python' in first_line.lower()): | ||
| log.info("No #!python executable found, skipping .bat " "wrapper") |
There was a problem hiding this comment.
| log.info("No #!python executable found, skipping .bat " "wrapper") | |
| log.info("No #!python executable found, skipping .bat wrapper") |
|
Hi @mcsitter. This has gotten a bit out-of-date with other things being changed, so I'm going to close for now. If you care to take another stab at this, I would suggest breaking it into independent chunks, which will be easier to review one at a time. |
Hello,
i want to contribute to this awesome project. Here is my first PR. Since this is my first OpenSource contribution i hope, that i will perform by expected standards.
I think there are a lot of Python2 files in this repo, which are of no use. One could dig into their function and clean up a bit. Some of it looked like Python2 to Python3 conversion. At the end of this year Python2 will not be supported anymore, so one could probably drop it.
Related Issues are #734 #735 and #803
To make getting started easier i added the environment.yml. It makes getting started really easy, if the dev chooses numpy. The script, which updates the requirements files won't update automatically, but i am sure that is no problem. The versions are not pinned, so the latest compatible will be chosen by standard.
In the doc/conf.py numpydoc is used as an extension. sphinx.ext.napoleon supports the numpydoc standard however. That would reduce that requirement and i couldn't find any issues with the change.
I want to explore the codebase and get started on improving the documentation. Sadly i couldn't find a formatting standard, aside from the flake8 ignores in the config file. Since they are not explained and are so many i would think, they are excluded out of convenience. Using the Black code-formatter and flake8 could improve code readability. Following standards could be a good idea.
I added the docstring-convention option to the flake8-config. It doesn't hurt and helps if you use flake8-docstrings.
I am open to discuss anything and would love some help, tips and guidance on this project!