fix: best practices and cleanups from automated review#1118
Open
henryiii wants to merge 3 commits into
Open
Conversation
- Replace mutable default storage args in numpy.py with None + lazy init - Fix AxesTuple.index return type from tuple[float,...] to tuple[int,...] - Fix Self = object fallback to Self = typing.Any in _compat/typing.py - Remove duplicate BHP_NOEXCEPT_17 macro in register_axis.hpp - Guard division by zero in mean.hpp and weighted_mean.hpp variance() - Remove leftover print() debug statements in test files - Remove commented-out dead test_hist_division code - Make TestBoolean inherit from Axis ABC for structural coverage - Add hypothesis.settings(deadline=None) to property-based test files Assisted-by: OpenCode:GLM-5
Assisted-by: CopilotCLI:glm-5.1 Signed-off-by: Henry Schreiner <henryfs@princeton.edu>
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.
🤖 Human triggerd, AI assisted PR. AI text below. 🤖
Addresses the Best Practices & Cleanups section of #1115.
Python fixes
storage=_storage.Double()withstorage=None+ lazy init inhistogramdd()andhistogram2d()AxesTuple.indexreturn type fromtuple[float, ...]totuple[int, ...]Self = objectfallback toSelf = typing.Any(more semantically correct)C++ fixes
BHP_NOEXCEPT_17macro definition (lines 43-47)variance()— returns NaN whencount <= 1variance()— returns NaN when denominator <= 0Test cleanups
print()debug statements fromtest_histogram.pyandtest_histogram_indexing.pytest_hist_divisioncode (from 2020)TestBooleaninherit fromAxisABC for structural test coveragehypothesis.settings(deadline=None)totest_pbt.pyandtest_accumulators.pySkipped
rebin.__init__validation: Investigated but existing logic is correct —axisalongsidegroups/edgesis a valid use case (confirmed by existing tests)using namespace pybind11::literalsin header: Too many usages across the C++ codebase to safely refactor in this PRCloses #1115 (best practices portion)
Assisted-by: OpenCode:GLM-5