Hotfix/style cleanup - #38
Merged
Merged
Conversation
check_docs.py carried its three checks in one 387-line file, separated by banner comments — the kind future contributors read past. Each check is now its own module under scripts/_check_docs, named after what it checks, and check_docs.py is the entry point that walks the documents and calls them. The module docstring still lists the three, now pointing at the module each lives in. `make test-docs-static` invokes it the same way and reports the same 151 claims across 20 documents. document.py holds what all three need: the Line/Report types, reading a document's code spans, and finding the documents. Its EXCLUDED comment said "see the module docstring" for why CHANGELOG.md is skipped, which was true in the old single file and dangling here, so it states the reason itself. Four functions renamed while they moved, since they are the ones the split made awkward: cli_flags() shadowed the module it now lives in, and makefile_targets(), documents() and a bare read() were nouns for things that go and fetch. They are read_cli_flags, read_makefile_targets, find_documents and read_lines. check_cli_flags opened with an assignment that the loop on the next line always overwrote. Dropped; finditer yields at least one match wherever the search that guards the branch matched. scripts/ had no importable package before this, so nothing ever left a __pycache__ there and no .gitignore covered it — the SDKs ignore bytecode in their own. A root rule now catches the rest of the tree.
client.py and provider.py divided their classes with banner comments — "-- public API ---", "-- execution ---", and twelve more. Every method under them already has a docstring saying what it does, so the banners were labelling groups that named themselves, and they are the first thing to go stale when a method moves. Fourteen removed, nothing else touched.
"New E2E tests for recently added features" sat above twenty test functions, which is every test in the second half of the file — it dated them rather than grouping them, and the features stopped being recent some releases ago. _BLUEFORS_CHANNELS was defined fifteen hundred lines in, immediately above the first helper that reads it. It joins the other module constants at the top. The file is not ruff-formatted and this does not change that; reformatting it would bury two lines of intent in six hundred lines of rewrapping.
…nd lift constants The four functions that paper over quantify-scheduler and qblox-scheduler disagreeing about a compiled schedule sat at the bottom of coordinator.py under a banner comment carrying the whole explanation — which of the two spells pulse_info as a list, where a subschedule keeps its operations, whether a drive amplitude is G_amp or amplitude. That is a module's worth of one subject, so it is simulation/_compiled_schedule.py now and the banner's prose is its docstring. Renamed on the way out, because the names only read inside the file that defined them: infos() said nothing about returning pulse or acquisition entries, and operations_of/amplitude_of were nouns. They are read_info_entries, read_operations, read_amplitude, and is_subschedule keeps its name. Constants moved to the top of their modules, out from between the functions that use them: MAX_ENTANGLED and the four drive-step and propagator-cache bounds in coordinator.py, MAX_BROADENING in lorentzian.py, _DEPARTED/_RETURNED in chevron.py, GRID_NS in tuners/base/routines.py, BIAS_KEYS in coupler_bias.py. Every comment above them moved too. The remaining banners in transmon.py, coupled.py, agent.py, dag.py and coordinator.py are gone. One was load-bearing — routines.py explained what a check is and why the two methods mirror build_schedule/analyse — so that went into build_check_schedule's docstring rather than the bin. Constants left where they are: DEVICE_SPEC in bluefors_gen1.py, DEVICE_SPECS in qpu.py and calibrate.py, DeviceBuilder and _DEVICES in registry.py. Each is built from a function or class defined above it and cannot move over its own dependency.
… real names tests/utils.py held fixture loading and qiskit gate-conversion assertions in one file, and tests/fixtures/ held two Python modules beside its data files. Helpers are now a tests/utils package split by subject: io.py loads the fixture data, circuits.py compares a converted gate against qiskit's own unitary, simulation.py holds the transmon test doubles, half_imported_device.py is still the module that fails to import. fixtures/ is data files only, and says so. The nine test modules that divided their tests with banner comments now use test classes. Each banner's title became the class name and its prose the class docstring, so what was a comment is now something pytest prints and a reader can find — TestTheWholeDagThroughTheRealStack rather than "--- the whole DAG, through the real stack ---". Helpers and constants that sat between the tests of a section move above the class they serve. Where a banner only restated the single docstring below it, it is simply gone. No test changed behaviour: pytest collects the same 739 items before and after, identical once class names are stripped from the node IDs, and the suite is 615 passed / 124 skipped either way. The skips are the second scheduler's parametrisation, as before. test_discovery.py's dotted paths for the moved module are four characters shorter, which let ruff rejoin a wrapped assertion.
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.
Why
Just cleaning up some python code smells.