Skip to content

Conversation

JamesWrigley
Copy link
Contributor

@JamesWrigley JamesWrigley commented Oct 18, 2025

These are already defined by Base in terms of other comparison operators, so they cause a ton of invalidations. I believe the Python operators have the same logical requirements as the corresponding Julia ones, so this should still be correct 🤞

The total number of invalidations on Julia 1.12 went down to 8551 from 11647 (not including the fixed invalidations from the UnsafePointers.jl and Julia PRs, those remove a few more thousand). These invalidations should be fixed:

 inserting >=(x::PythonCall.Py, y::PythonCall.Py) @ PythonCall.Core ~/git/PythonCall.jl/src/Core/Py.jl:370 invalidated:                                                                                                                        
   backedges: 1: superseding >=(x, y) @ Base operators.jl:472 with MethodInstance for >=(::Any, ::Any) (2 children)

 inserting >=(x::Number, y::PythonCall.Py) @ PythonCall.Core ~/git/PythonCall.jl/src/Core/Py.jl:389 invalidated:                                                                                                                               
   backedges: 1: superseding >=(x, y) @ Base operators.jl:472 with MethodInstance for >=(::Int64, ::Any) (10 children)

 inserting >(x::Number, y::PythonCall.Py) @ PythonCall.Core ~/git/PythonCall.jl/src/Core/Py.jl:390 invalidated:
   backedges: 1: superseding >(x, y) @ Base operators.jl:425 with MethodInstance for >(::Int64, ::Any) (2 children)
              2: superseding >(x, y) @ Base operators.jl:425 with MethodInstance for >(::Int64, ::Any) (43 children)

 inserting !=(x::Number, y::PythonCall.Py) @ PythonCall.Core ~/git/PythonCall.jl/src/Core/Py.jl:386 invalidated:
   backedges: 1: superseding !=(x, y) @ Base operators.jl:321 with MethodInstance for !=(::Int64, ::Any) (69 children)

 inserting >(x::PythonCall.Py, y::PythonCall.Py) @ PythonCall.Core ~/git/PythonCall.jl/src/Core/Py.jl:371 invalidated:
   backedges: 1: superseding >(x, y) @ Base operators.jl:425 with MethodInstance for >(::Any, ::Any) (225 children)

 inserting !=(x::PythonCall.Py, y::PythonCall.Py) @ PythonCall.Core ~/git/PythonCall.jl/src/Core/Py.jl:367 invalidated:
   backedges: 1: superseding !=(x, y) @ Base operators.jl:321 with MethodInstance for !=(::Any, ::Any) (377 children)

 inserting >(x::PythonCall.Py, y::Number) @ PythonCall.Core ~/git/PythonCall.jl/src/Core/Py.jl:381 invalidated:
   backedges: 1: superseding >(x, y) @ Base operators.jl:425 with MethodInstance for >(::Any, ::Bool) (8 children)
              2: superseding >(x, y) @ Base operators.jl:425 with MethodInstance for >(::Any, ::Int128) (8 children)
              3: superseding >(x, y) @ Base operators.jl:425 with MethodInstance for >(::Any, ::UInt128) (8 children)
              4: superseding >(x, y) @ Base operators.jl:425 with MethodInstance for >(::Any, ::Int64) (8 children)
              5: superseding >(x, y) @ Base operators.jl:425 with MethodInstance for >(::Any, ::UInt64) (17 children)
              6: superseding >(x, y) @ Base operators.jl:425 with MethodInstance for >(::Any, ::UInt8) (56 children)
              7: superseding >(x, y) @ Base operators.jl:425 with MethodInstance for >(::Any, ::Integer) (66 children)
              8: superseding >(x, y) @ Base operators.jl:425 with MethodInstance for >(::Any, ::Int64) (209 children)

 inserting >=(x::PythonCall.Py, y::Number) @ PythonCall.Core ~/git/PythonCall.jl/src/Core/Py.jl:380 invalidated:
   backedges: 1: superseding >=(x, y) @ Base operators.jl:472 with MethodInstance for >=(::Any, ::Int64) (2 children)
              2: superseding >=(x, y) @ Base operators.jl:472 with MethodInstance for >=(::Any, ::UInt64) (3 children)
              3: superseding >=(x, y) @ Base operators.jl:472 with MethodInstance for >=(::Any, ::Int64) (408 children)

 inserting !=(x::PythonCall.Py, y::Number) @ PythonCall.Core ~/git/PythonCall.jl/src/Core/Py.jl:377 invalidated:
   backedges: 1: superseding !=(x, y) @ Base operators.jl:321 with MethodInstance for !=(::Any, ::Bool) (14 children)
              2: superseding !=(x, y) @ Base operators.jl:321 with MethodInstance for !=(::Any, ::UInt8) (32 children)
              3: superseding !=(x, y) @ Base operators.jl:321 with MethodInstance for !=(::Any, ::Int64) (375 children)

@JamesWrigley
Copy link
Contributor Author

JamesWrigley commented Oct 18, 2025

Not sure what's going on with the CI failures 🤔

@cjdoris
Copy link
Collaborator

cjdoris commented Oct 18, 2025

Not sure what's going on with the CI failures 🤔

Also not sure, looks like something has broken PyCall (a test dependency) in CI. Unrelated to this though.

@cjdoris
Copy link
Collaborator

cjdoris commented Oct 18, 2025

This is a tricky one because on the one hand, these definitions do need to stay as they are because Python is weird. For example x == y in Python doesn't have to return a bool, and certainly (x != y) is allowed to be different from not (x == y).

On the other hand, these comparison operators return a Py which violates the docstring for == (which I only just read and which explicitly says the result is a Bool unless one argument is a Missing) so these are illegal methods to start with. They are puns, really.

One can view this latter point as a bug and therefore fixable in a technically non-breaking fashion. But it definitely will break people's code. We're working (extremely slowly) towards PythonCall v1 which can definitely fix these issues.

The right thing to do would be to replace all these methods with ones that correctly return Bool (not Py) and remove the unnecessary ones in this PR (hence using the generic fallbacks in Base). I'll ponder if we can/should do this pre-v1.

@cjdoris
Copy link
Collaborator

cjdoris commented Oct 18, 2025

Thinking some more, actually I think these methods are not documented except there is one example which implies that ==(::Py, ::Int) returns Py.

So perhaps it's ok to change these methods in a non-breaking release since the behaviour is (almost) undocumented, and is incompatible with the definition of Base.== anyway.

However like I said, this change will quite likely break a bunch of code in practice.

@JamesWrigley
Copy link
Contributor Author

Mmmm, interesting. I feel like it's probably safer to put this in a breaking release then.

@cjdoris cjdoris changed the base branch from main to v1 October 19, 2025 08:38
@cjdoris
Copy link
Collaborator

cjdoris commented Oct 19, 2025

I've just changed the target branch from main to v1 but now this PR has a bunch of commits on main that aren't merged into v1 yet. I'll do that later.

These are already defined by Base in terms of other comparison operators, so
they cause a ton of invalidations.
@JamesWrigley
Copy link
Contributor Author

I rebased it :) Do you know when v1 (or an RC) will be released?

@cjdoris
Copy link
Collaborator

cjdoris commented Oct 19, 2025

I rebased it :) Do you know when v1 (or an RC) will be released?

Thanks. I'm not sure but there's a milestone to track it: https://github.com/JuliaPy/PythonCall.jl/milestone/1

It's been in the works for a very long time but I'm actually managing to chip away at it at the moment. There are two major PRs to do, one of which is basically ready and one is unstarted but the thinking has been done. Then there are a bunch of little things.

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.

2 participants