Repository navigation
Conversation
read_idealized_bursts and log_bin_edges import dcio and scalcs at call time, so the likelihood library needs nothing but numpy. The cost of that design is that the CI matrix, which installs neither, could not exercise either entry point -- so both shipped with no coverage at all, and a green run said nothing about them. #180's delegation was verified only on my machine. Adds a dcio-integration job that installs both from git and runs tests against an installed package rather than the checkout. It builds through scikit-build-core, which is also what a user gets. The tests pin golden values for the shipped CH82 record (572 bursts, 1100 intervals, 4.819128 s), that every burst has an odd interval count, that the sign of tcrit does not change segmentation, the bins-per-decade rule, and that the histogram bars account for every interval. One of them matters more than the rest: importing HJCFIT in a subprocess must leave dcio and scalcs out of sys.modules, which is the property the lazy imports exist to provide and the one a future refactor is most likely to break. Also corrects the ImportError guidance. It pointed at github.com/remislp/dcio and github.com/remislp/SCALCS; both moved to the DCPROGS organisation and the SCALCS fork has been deleted, so the only advice the error gave was a dead link. It now gives the two pip commands. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
An escaped newline collapsed into a real one while editing, splitting the string literal across lines. python/__init__.py then failed to compile, which ctest caught as python_qmatrix and every matrix job reported. It passed locally because 'import HJCFIT' resolves to the installed package, not the source tree, so nothing I ran locally ever compiled the file I had edited. Compiling the sources directly is the check that catches this. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
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.
read_idealized_burstsandlog_bin_edgesimport dcio and scalcs at calltime, so the likelihood library needs nothing but numpy. The cost of that
design is that the CI matrix — which installs neither — could not exercise
either entry point. Both shipped with no coverage at all, and #180's green
7/7 said nothing about the delegation it introduced. It was verified only on my
machine.
What this adds
A
dcio-integrationjob that installs both from git and runs tests against aninstalled package rather than the checkout. It builds through
scikit-build-core, which is also what a user gets.
12 tests. The golden ones pin the shipped CH82 record — 572 bursts, 1100
intervals, 4.819128 s total — plus the bins-per-decade rule and that the
histogram bars account for every interval.
Three pin properties rather than numbers:
likelihood is a product of matrices alternating A→F and F→A, and an
even-length burst would end on a shut;
tcritdoes not change segmentation — negative is a flag toLog10Likelihoodselecting equilibrium over CHS vectors, and the same numberis passed to both;
sys.modules, checked in asubprocess. That is the property the lazy imports exist to provide and the one
a future refactor is most likely to break silently.
Also
The
ImportErrorguidance pointed atgithub.com/remislp/dcioandgithub.com/remislp/SCALCS. Both moved to the DCPROGS organisation, and theSCALCS fork was deleted — so the only advice the error offered was a dead link.
It now gives the two
pip installcommands.Not changed
The six existing matrix jobs still install neither, which is what keeps proving
the soft dependency actually holds.
🤖 Generated with Claude Code