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

chore: reformat multi-line statements #2379

Merged
merged 2 commits into from
Nov 25, 2024

Conversation

mwtoews
Copy link
Contributor

@mwtoews mwtoews commented Nov 24, 2024

This PR refactors some multi-line statements. This was mostly automated by temporarily toggling this control in pyproject.toml:

[tool.ruff.format]
skip-magic-trailing-comma = true

then selecting some commits that condense multi-line statements to single, while rejecting other statements that are better represented over multiple lines. There was no exact rule-of-thumb for the process, except to try and maintain the same style in surrounding code blocks. Class constructors or instantiations of some classes with many parameters are kept as multi-line.

Note that while there are over 2300 fewer lines with this PR, there are no functional changes.

Copy link

codecov bot commented Nov 24, 2024

Codecov Report

Attention: Patch coverage is 67.32673% with 33 lines in your changes missing coverage. Please review.

Project coverage is 75.9%. Comparing base (bb9824e) to head (a3abadf).
Report is 11 commits behind head on develop.

Files with missing lines Patch % Lines
flopy/utils/gridgen.py 66.6% 3 Missing ⚠️
flopy/mfusg/mfusglpf.py 60.0% 2 Missing ⚠️
flopy/modflow/mfbas.py 0.0% 2 Missing ⚠️
flopy/modflow/mfbct.py 0.0% 2 Missing ⚠️
flopy/modflow/mflpf.py 0.0% 2 Missing ⚠️
flopy/modflow/mfrch.py 0.0% 2 Missing ⚠️
flopy/modflow/mfupw.py 33.3% 2 Missing ⚠️
flopy/modpath/mp6sim.py 50.0% 2 Missing ⚠️
flopy/export/metadata.py 0.0% 1 Missing ⚠️
flopy/mbase.py 50.0% 1 Missing ⚠️
... and 14 more
Additional details and impacted files
@@            Coverage Diff            @@
##           develop   #2379     +/-   ##
=========================================
+ Coverage     68.4%   75.9%   +7.5%     
=========================================
  Files          294     294             
  Lines        59390   61639   +2249     
=========================================
+ Hits         40652   46842   +6190     
+ Misses       18738   14797   -3941     
Files with missing lines Coverage Δ
flopy/__init__.py 100.0% <ø> (ø)
flopy/discretization/structuredgrid.py 48.7% <100.0%> (+1.5%) ⬆️
flopy/discretization/unstructuredgrid.py 81.6% <100.0%> (+6.5%) ⬆️
flopy/export/netcdf.py 49.2% <100.0%> (+0.6%) ⬆️
flopy/export/shapefile_utils.py 84.0% <100.0%> (+3.4%) ⬆️
flopy/export/utils.py 64.6% <ø> (+11.9%) ⬆️
flopy/mfusg/__init__.py 100.0% <ø> (ø)
flopy/mfusg/cln_dtypes.py 75.0% <ø> (+33.3%) ⬆️
flopy/mfusg/mfusg.py 77.5% <100.0%> (+11.2%) ⬆️
flopy/mfusg/mfusgbcf.py 80.4% <ø> (+71.6%) ⬆️
... and 74 more

... and 166 files with indirect coverage changes

---- 🚨 Try these New Features:

@wpbonelli wpbonelli merged commit 080b965 into modflowpy:develop Nov 25, 2024
23 checks passed
@mwtoews mwtoews deleted the reformat-multi-line-statements branch November 25, 2024 16:56
Copy link
Contributor Author

@mwtoews mwtoews left a comment

Choose a reason for hiding this comment

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

Triple-check of changes complete, with a few notes below. This PR highlighted a few places where a trailing , was originally used, sometimes to try and construct a tuple type. I'll follow this up with a possible correction and amend .git-blame-ignore-revs.

Comment on lines -931 to +926
if i == 0 or nstrm > 0 and not reachinput or isfropt <= 1:
if i == 0 or (nstrm > 0 and not reachinput) or isfropt <= 1:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

FYI, this one was an autofix for RUF021

Comment on lines -151 to +147
shape = tuple(
nodes,
)
shape = tuple(nodes)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should probably be corrected to shape = (nodes,) (it was probably originally shape = tuple(nodes,) which is not correct)

delr=np.ones(
self.ncol,
),
delr=np.ones(self.ncol),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is where the code was probably originally np.ones(self.ncol,) in attempt to specify a tuple (which would be done with np.ones((self.ncol,))). Fortunately, this is harmless, as numpy's shape can be an integer to mean the same as a tuple of one integer. No change is recommended here.

Comment on lines +382 to +383
top = np.ones((nnodes))
botm = np.ones((nnodes))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This probably should be corrected to np.ones((nnodes,)) (or simply np.ones(nnodes))

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