fix: correct class name in Long{AboveMax,BelowMin} type-change error#3634
Open
anxkhn wants to merge 1 commit into
Open
fix: correct class name in Long{AboveMax,BelowMin} type-change error#3634anxkhn wants to merge 1 commit into
anxkhn wants to merge 1 commit into
Conversation
LongAboveMax.to() and LongBelowMin.to() raised a TypeError whose message named IntAboveMax/IntBelowMin, a copy-paste leftover from the Int singletons above them. When a long overflow literal is asked to convert to an unsupported type, the diagnostic misidentified it as an int literal, which is misleading when debugging. Point each message at its own class and add regression tests covering both the successful conversion and the corrected error message for the long singletons.
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.
Rationale for this change
LongAboveMax.to()andLongBelowMin.to()fall back toraise TypeError("Cannot change the type of ...")when asked to convert to anunsupported type, but the class name in each message was left as
IntAboveMax/IntBelowMin, a copy-paste leftover from theIntAboveMax/IntBelowMinsingletons defined just above. So when a long overflow sentinel is converted to
an unsupported type, the diagnostic misidentifies it as an int literal, which is
misleading when debugging.
For comparison, the neighbouring sentinels already name themselves correctly:
FloatAboveMax/FloatBelowMin,IntAboveMax/IntBelowMin. Only the twoLong*messages are wrong.This is the same class of copy-paste-from-the-
Int-handler slip thatDecimalLiteral.to(LongType)had in #3469 (fixed by #3470, which corrected thereturned sentinel object); this PR fixes the remaining instance in the
diagnostic messages on the
Long*singletons themselves.Change: point each message at its own class:
pyiceberg/expressions/literals.pyLongAboveMax.to:IntAboveMax->LongAboveMaxpyiceberg/expressions/literals.pyLongBelowMin.to:IntBelowMin->LongBelowMinAre these changes tested?
Yes. Added
test_above_max_longandtest_below_min_longintests/expressions/test_literals.py, mirroring the existingtest_above_max_int/
test_below_min_int(singleton identity,str/repr,value, the successful.to(LongType())round-trip) and additionally asserting that.to(IntegerType())raises
TypeErrorwhose message now names theLong*class.Verified red -> green: reverting only the source change makes both new tests fail
with
'Cannot change the type of LongBelowMin' in 'Cannot change the type of IntBelowMin'; with the fix, both pass.tests/expressions/test_literals.py: 172passed.
tests/expressions/: 566 passed, no regressions.make lint(ruff,ruff-format, mypy, pydocstyle, codespell, license/header) passes on the changed
files.
Are there any user-facing changes?
No API or behavior change. Only the text of an already-raised
TypeErroriscorrected (the exception type and the conditions under which it is raised are
unchanged). No changelog entry needed.