Import numpy, pandas and matplotlib under their conventional aliases - #98
Merged
Conversation
Clears ICN001 from the ignore list (issue #40), taking it from 59 entries to 58. Nineteen imports and 262 usage sites across 12 files. The package was already inconsistent here: export_vector.py and plot_photon_clouds_v2.py used np and pd, and the latter aliased numpy and pandas while leaving matplotlib bare. This settles it the way the rule and the wider ecosystem expect. The rewrite targets ast.Name node positions, which by construction cover only real identifier references, so comments, docstrings and the import statements themselves were left alone. That matters here: twenty mentions sit in commented-out code, and five sit in docstrings where the full name is the correct prose form -- "returns a pandas.DataFrame" should not become "pd.DataFrame". `import matplotlib.pyplot as plt` and `from matplotlib import ticker` are untouched, since only bare `import matplotlib` was flagged. Equivalence was proven rather than inspected: both the old and new ASTs were normalised to a canonical form, with aliases expanded back to full module names and alias bindings stripped, and compared. All twelve files match. Since docstrings are AST constants, that also proves none were altered, and a separate check confirmed no comment line changed. Two knock-on effects came from the names simply getting shorter. An if/else in validate_dem.py that was previously too long to express as a ternary now fits, so SIM108 -- already enabled -- flagged it; reverting just that ternary makes the file's AST match too, confirming it is the only non-alias change in the diff. The formatter also repacked one file whose wrapped expressions now fit on single lines.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Clears
ICN001from the Ruff ignore list, which goes from 59 entries to 58. Progress on #40.19 imports and 262 usage sites across 12 files — 207 numpy, 69 pandas, 12 matplotlib identifier tokens in total, less the 26 that legitimately remain on import lines.
Why now
The package was already inconsistent:
export_vector.pyandplot_photon_clouds_v2.pyusednp/pd, and the latter aliased numpy and pandas while leavingimport matplotlibbare. This wasn't a house style being defended, just drift, so it's settled the way the rule and the wider ecosystem expect.How the rewrite was done
It targets
ast.Namenode positions, which by construction cover only real identifier references — never comments, docstrings, or the import statements. That distinction matters here:# center = numpy.mean(...)). These overlap heavily withERA001(425 still ignored); that text may simply be deleted later, so rewriting it now would be wasted work.pandas.DataFrame" should not become "pd.DataFrame".import matplotlib.pyplot as pltandfrom matplotlib import tickerare untouched, since only bareimport matplotlibwas ever flagged. The awkwardmpl.use("Agg")sitting between imports inplot_photon_clouds_v2.pycame through correctly.Equivalence was proven, not inspected
Both the old and new ASTs were normalised to a canonical form — aliases expanded back to full module names, alias bindings stripped — and compared with
ast.dump(). All 12 files match. Because docstrings are AST constants, that also proves none were altered, and a separate diff check confirmed no comment line changed.Two knock-on effects, both from names getting shorter
SIM108fired: anif/elseinvalidate_dem.pythat was previously too long to express as a ternary now fits, so the already-enabled rule flagged it. Reverting just that ternary makes the file's AST match as well, confirming it is the only non-alias change anywhere in the diff.This is the same shape as the
RET504that appeared in #97, and is worth expecting again on thePTH*family.Verification
ruff check,ruff formatand the fullpreksuite pass. All touched modules import cleanly andivert --helpworks, with one exception noted below.Note
plot_results_slope_centrality.pydoes not import at all — it doesimport import_parent_dir, a module that does not exist. This is pre-existing and fails identically onmain; it is not caused by this change. The file is included here only so the rule could be enabled, and is slated for removal separately.No changelog entry: this is an internal refactor with no user-visible effect.
🔍 Docs preview: https://ivert--98.org.readthedocs.build/en/98/