Remove self-imports that existed because of imp.load_module(fp) - #2017
Merged
Conversation
ModuleGather.getModule used to load each file as a separate top-level module, so a test's own module globals held a different copy of its classes than music21.spanner did. Tests worked around it by re-importing their own module -- `from music21 import spanner` inside spanner.Test -- which restored identity but also made the module read as though it were a foreign one. Now that the runner imports by fully-qualified name (#2014), the workaround is dead weight. Removes 90 self-imports across 21 modules and rewrites their references to the bare module-level names. Also drops three comments that only explained the workaround (bar.py's "avoid not same class error" plus its pylint disable, parallel.py's "we need the full path to the modules", iterator.py's noinspection), and re-aligns three continuation lines the shortened call names left over-indented. Not touched: clef.clefFromString's `from music21 import clef as myself`, which is a real dynamic lookup over the module's own namespace, not a test artifact. Verified with multiprocessTest (5299 tests) and testSingleCoreAll (5022), plus ruff, mypy, pylint, and a pycodestyle E12 pass held to its prior count. AI-assisted (Claude)
clefFromString imported its own module to enumerate the clef classes by name; globals() is the same namespace without the import. The doctests in configure.Dialog._rawQueryPrepareHeader and tempo.TempoText imported music21 (or configure) before using it, which the pytest plugin already supplies -- it injects music21 and everything in its __all__ into the doctest namespace. base.py keeps its four `>>> import music21` lines, which are there to show the fully-qualified path rather than to make the example run. TempoText's docstring was a bare example with no prose; says now what the class is for. AI-assisted (Claude)
The name scan `if 'Clef' not in className` also matched ClefException, so
clefFromString('clefexception') built and returned an exception object. Filter
on issubclass(Clef) instead; every clef class in the module ends in 'Clef', so
the clefs found are unchanged.
AI-assisted (Claude)
clef.py is the only file in this branch whose change stands on its own -- the other twenty are a mechanical removal of dead imports, and bumping twenty copyright banners would bury the diff under them. AI-assisted (Claude)
|
coverage: 93.337% (-0.003%) from 93.34% — self-imports into master |
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
2 participants
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.
#2014 fixed the import mechanism used by tests. Before if a test was in a file (say key.py) it could not use a class like
KeySignatureunguarded becauseisinstancechecks andrepr()was different. So our tests in the same module used to begin withfrom music21.my_own_module import MyOwnClassNo longer needed. This is cleanup that was kept out of #2014 to focus that PR on the mechanism.
Also update rules for copyright bumping. Add a missing doc for TempoText. Better class filtering in clefFromString
AI-Assisted (Claude)