Align style checks with jwst #383
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #383 +/- ##
===========================================
+ Coverage 67.59% 78.17% +10.57%
===========================================
Files 115 115
Lines 5932 5145 -787
===========================================
+ Hits 4010 4022 +12
+ Misses 1922 1123 -799 ☔ View full report in Codecov by Sentry. |
| "UP015", # unnecessary open(file, "r"). no harm being explicit | ||
| "TRY003", # prevents custom exception messages not defined in exception itself. | ||
| "ISC001", # single line implicit string concatenation. formatter recommends ignoring this. | ||
| "PTH123", # use Path.open instead of open |
There was a problem hiding this comment.
There are lots of open uses in the code. I don't see why Path(filename).open is preferred.
| "TRY003", # prevents custom exception messages not defined in exception itself. | ||
| "ISC001", # single line implicit string concatenation. formatter recommends ignoring this. | ||
| "PTH123", # use Path.open instead of open | ||
| "UP038", # isinstance with | instead of , |
There was a problem hiding this comment.
As far as I'm aware isinstance(foo, FooType | BarType) and isinstance(foo, (FooType, BarType)) are equivalent and the latter is faster.
| "B904", # raise ... from ?, when and where do we want to not use the default chain? | ||
| "NPY002", # numpy rng, will require finding the right replacements | ||
| "S101", # asserts are used in many non-test places | ||
| "SLF001", # private member access, this is overly restrictive |
There was a problem hiding this comment.
I put this in the "fix later" category but I'm not convinced it's worth the effort for the number of what I would consider "false positives".
| - id: ruff | ||
| args: ["--fix"] | ||
| - id: ruff-format | ||
| # - repo: https://github.com/numpy/numpydoc |
There was a problem hiding this comment.
Even after excluding test.* items this shows 2884 failures. My vote is to enable and attempt to address these in a follow up PR.
braingram
force-pushed
the
style_checks
branch
from
fe3d7b5 to
1ce97af
Compare
spacetelescope/jwst#9081 introduced major changes to the code style checks in jwst.
Since this package primarily serves jwst this PR updates the style checks here to more closely match those in jwst.
Regtests: https://github.com/spacetelescope/RegressionTests/actions/runs/13039378253
show 1 error
test_exec_time_0_crswhich I'm chalking up to a slower-than-usual runner which took 11 instead of 10 seconds. I expect these may need to be re-run after review so I'll open this PR for review now.Branch protections will need to be updated after this PR is merged to check for pre-commit success.
Tasks
docs/pageno-changelog-entry-needed)changes/:echo "changed something" > changes/<PR#>.<changetype>.rst(see below for change types)jwstregression tests with this branch installed ("git+https://github.com/<fork>/stdatamodels@<branch>")news fragment change types...
changes/<PR#>.feature.rst: new featurechanges/<PR#>.bugfix.rst: fixes an issuechanges/<PR#>.doc.rst: documentation changechanges/<PR#>.removal.rst: deprecation or removal of public APIchanges/<PR#>.misc.rst: infrastructure or miscellaneous change