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

Fix various code style issues #165

Merged
merged 9 commits into from
Jul 30, 2024
Merged

Fix various code style issues #165

merged 9 commits into from
Jul 30, 2024

Conversation

imagejan
Copy link
Member

@imagejan imagejan commented Jul 29, 2024

Most importantly, don't use module names that are the same as the contained class names, as this can lead to errors where the import order matters, as classes are confused with their modules. (For example, this was the case if you sorted imports of hcs/imagexpress/__init__.py before this commit.)

We now collect several classes into single (sub-)modules.

Usages of xml/lxml were replaced by alternatives from defusedxml.

This commit also fixes various other errors reported by hatch fmt (i.e. ruff).


I didn't rename any of the test files yet, although they should also be lower-case in general, and correlate with the now-renamed modules. I'll leave this for a follow-up pull request.


Closes #117.

Most importantly, don't use module names that are the same as the contained class names,
as this can lead to errors where the import order matters, as classes are confused with their modules.
(For example, this was the case if you sorted imports of hcs/imagexpress/__init__.py before this commit.)

We now collect several classes into single (sub-)modules.

Usages of `xml`/`lxml` where replaced by alternatives from `defusedxml`.

This commit also fixes various other errors reported by `hatch fmt` (i.e. ruff).
Copy link

codecov bot commented Jul 29, 2024

Codecov Report

Attention: Patch coverage is 92.98532% with 43 lines in your changes missing coverage. Please review.

Project coverage is 91.50%. Comparing base (47969d6) to head (d008b11).

Files Patch % Lines
src/faim_ipa/visiview/acquisition.py 54.16% 11 Missing ⚠️
src/faim_ipa/visiview/ome_companion_utils.py 56.25% 7 Missing ⚠️
src/faim_ipa/mobie.py 0.00% 6 Missing ⚠️
src/faim_ipa/hcs/cellvoyager/acquisition.py 97.87% 2 Missing and 2 partials ⚠️
src/faim_ipa/alignment/alignment.py 66.66% 2 Missing and 1 partial ⚠️
src/faim_ipa/hcs/acquisition.py 83.33% 2 Missing and 1 partial ⚠️
src/faim_ipa/zarr_utils.py 0.00% 3 Missing ⚠️
src/faim_ipa/hcs/imagexpress/acquisition.py 98.71% 1 Missing and 1 partial ⚠️
src/faim_ipa/histogram.py 66.66% 1 Missing and 1 partial ⚠️
src/faim_ipa/utils.py 94.28% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main     #165       +/-   ##
===========================================
+ Coverage   81.03%   91.50%   +10.47%     
===========================================
  Files          35       50       +15     
  Lines        1687     3295     +1608     
  Branches      240      345      +105     
===========================================
+ Hits         1367     3015     +1648     
+ Misses        314      270       -44     
- Partials        6       10        +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@imagejan imagejan force-pushed the code-style-refactor branch 2 times, most recently from 64fea52 to c9fc096 Compare July 30, 2024 06:59
@imagejan
Copy link
Member Author

The remaining ruff errors are now:

src\faim_ipa\dask_utils.py:16:18: SLF001 Private member accessed: `_NoValue`
src\faim_ipa\dask_utils.py:18:15: SLF001 Private member accessed: `_NoValue`
src\faim_ipa\hcs\cellvoyager\acquisition.py:333:52: PLR2004 Magic value used in comparison, consider replacing `2` with a constant variable
src\faim_ipa\hcs\cellvoyager\acquisition.py:377:40: PLR2004 Magic value used in comparison, consider replacing `14` with a constant variable
src\faim_ipa\hcs\cellvoyager\acquisition.py:383:40: PLR2004 Magic value used in comparison, consider replacing `12` with a constant variable
src\faim_ipa\hcs\cellvoyager\acquisition.py:390:40: PLR2004 Magic value used in comparison, consider replacing `8` with a constant variable
src\faim_ipa\hcs\cellvoyager\acquisition.py:401:40: PLR2004 Magic value used in comparison, consider replacing `7` with a constant variable
src\faim_ipa\hcs\converter.py:70:9: PT018 Assertion should be broken down into multiple parts
src\faim_ipa\hcs\converter.py:155:16: PLR2004 Magic value used in comparison, consider replacing `2` with a constant variable
src\faim_ipa\hcs\converter.py:155:36: PLR2004 Magic value used in comparison, consider replacing `3` with a constant variable
src\faim_ipa\hcs\converter.py:258:41: SLF001 Private member accessed: `_store`
src\faim_ipa\hcs\converter.py:258:41: SLF001 Private member accessed: `_dimension_separator`
src\faim_ipa\hcs\converter.py:307:45: SLF001 Private member accessed: `_store`
src\faim_ipa\hcs\converter.py:307:45: SLF001 Private member accessed: `_dimension_separator`
src\faim_ipa\hcs\converter.py:346:31: PLR2004 Magic value used in comparison, consider replacing `2` with a constant variable
src\faim_ipa\hcs\converter.py:351:33: PLR2004 Magic value used in comparison, consider replacing `3` with a constant variable
src\faim_ipa\hcs\imagexpress\acquisition.py:186:18: SLF001 Private member accessed: `_files`
src\faim_ipa\stitching\stitching_utils.py:211:30: PLR2004 Magic value used in comparison, consider replacing `2` with a constant variable
src\faim_ipa\stitching\stitching_utils.py:221:30: PLR2004 Magic value used in comparison, consider replacing `2` with a constant variable
src\faim_ipa\utils.py:74:11: DTZ005 `datetime.datetime.now()` called without a `tz` argument
Found 20 errors.

We should discuss whether we want to fix or ignore them, I'll open a separate issue.


I will merge this, bump the version to 0.4.0 (as we have some breaking API changes) and release.

@imagejan imagejan merged commit 66e3607 into main Jul 30, 2024
12 checks passed
@imagejan imagejan deleted the code-style-refactor branch July 30, 2024 07:56
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.

Use lower-case module names everywhere
1 participant