Skip to content

Remove > and >= methods on Py to reduce invalidations - #829

Closed
MilesCranmerBot wants to merge 1 commit into
JuliaPy:mainfrom
MilesCranmerBot:no-gt-ge-py-overloads
Closed

MilesCranmerBot wants to merge 1 commit into
JuliaPy:mainfrom
MilesCranmerBot:no-gt-ge-py-overloads

Conversation

@MilesCranmerBot

Copy link
Copy Markdown
Contributor

Removes the > and >= methods on Py (for Py,Py, Py,Number, and Number,Py), as proposed in #828 (comment).

Why. Base defines > and >= only as generic fallbacks (y < x, y <= x), so calls like x > 1 with an abstractly typed x get inferred against PythonCall's methods, and defining them invalidates thousands of method instances elsewhere. Measured with SnoopCompile on Julia 1.12.7 (loading PythonCall + SymbolicRegression): 6830 → 2532 unique invalidated method instances, ~63% fewer from PythonCall.

Behavior. x > y now evaluates as Python y < x, i.e. Python tries y.__lt__ before x.__gt__ — only observable for types where __gt__ and __lt__ disagree. Results and the Py return type are otherwise unchanged; verified locally on Julia 1.10 with the fallbacks resolving to the existing </<= methods (Py(3) > 2, 2 > Py(3), Py(3) >= 3, 4 >= Py(3), Py(3) > Py(1), Py(3) > 2.5, …). pygt/pyge remain exported and are still used by the @py macro.

!= is intentionally untouched on 0.9: its Base fallback !(x == y) requires !(::Py), which does not exist (details in #828). The full fix remains the v1 branch (#702, #709).

Refs #828

Base only defines > and >= as generic fallbacks (y < x, y <= x), so
adding these methods invalidated thousands of method instances
elsewhere (6830 -> 2532 unique instances when loading PythonCall +
SymbolicRegression on Julia 1.12). Dropping them routes x > y through
Base's fallback to the existing < and <= methods, so results and the
Py return type are unchanged. The only behavior change is that Python
now tries y.__lt__ before x.__gt__.

pygt and pyge remain exported and are still used by the @py macro.

Refs JuliaPy#828

Co-authored-by: Miles Cranmer <miles.cranmer@gmail.com>
@MilesCranmerBot

Copy link
Copy Markdown
Contributor Author

Closing in favor of #830, which is the same change from the properly named branch (and supersedes this early duplicate).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant