sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.41k stars 475 forks source link

lazy_import breaks CachedRepresentation #19628

Closed cheuberg closed 6 years ago

cheuberg commented 8 years ago

NN is a lazy_import and therefore we have

sage: NonNegativeIntegerSemiring() == NN
False

but

sage: NN == NonNegativeIntegerSemiring()
True
sage: NonNegativeIntegerSemiring() == NN._get_object()
True

This gives problems with CachedRepresentation:

sage: from sage.misc.lazy_import import LazyImport
sage: lazyZZ = LazyImport('sage.rings.integer_ring', 'ZZ')
sage: PolynomialRing(lazyZZ, 'x') is PolynomialRing(ZZ, 'x')
False

Depends on #23102

CC: @vbraun @jdemeyer @mezzarobba @simon-king-jena @nthiery

Component: coercion

Author: Jeroen Demeyer

Branch: 866eddd

Reviewer: Erik Bray

Issue created by migration from https://trac.sagemath.org/ticket/19628

cheuberg commented 8 years ago
comment:1

The attached branch is the one which exhibits the error.

jdemeyer commented 8 years ago

Description changed:

--- 
+++ 
@@ -1,11 +1,27 @@
 In #19152, a `lazy_import` of `RBF` and `RealBallField` led to the error

-File "src/sage/rings/real_arb.pyx", line 314, in sage.rings.real_arb.RealBallField -Failed example:

jdemeyer commented 8 years ago

Description changed:

--- 
+++ 
@@ -1,27 +1,6 @@
 In #19152, a `lazy_import` of `RBF` and `RealBallField` led to the error

-sage: polygen(RBF, x) -x -sage: RBF(1) + polygen(QQ, x)

-TypeError Traceback (most recent call last) - in () -----> 1 RBF(Integer(1)) + polygen(QQ, x)

-/usr/local/src/sage-git/src/sage/structure/element.pyx in sage.structure.element.RingElement.add (build/cythonized/sage/structure/element.c:15967)()

jdemeyer commented 8 years ago

Changed dependencies from #19152 to none

jdemeyer commented 8 years ago

Description changed:

--- 
+++ 
@@ -1,6 +1,15 @@
 In #19152, a `lazy_import` of `RBF` and `RealBallField` led to the error

-sage: PolynomialRing(RealBallField(), 'x') is PolynomialRing(RBF, 'x') -False +sage: D = dict(); D[RealBallField()] = 1; D[RBF] = 2; D +{Real ball field with 53 bits precision: 2,

jdemeyer commented 8 years ago

Description changed:

--- 
+++ 
@@ -1,15 +1,6 @@
 In #19152, a `lazy_import` of `RBF` and `RealBallField` led to the error

-sage: D = dict(); D[RealBallField()] = 1; D[RBF] = 2; D -{Real ball field with 53 bits precision: 2,

jdemeyer commented 8 years ago

Description changed:

--- 
+++ 
@@ -1,6 +1,6 @@
-In #19152, a `lazy_import` of `RBF` and `RealBallField` led to the error
+`NN` is a lazy_import and therefore we have

-sage: RealBallField() == RBF +sage: NonNegativeIntegerSemiring() == NN False

jdemeyer commented 8 years ago
comment:9

Should we special-case LazyImport in WithEqualityById?

jdemeyer commented 8 years ago

Description changed:

--- 
+++ 
@@ -4,3 +4,9 @@
 sage: NonNegativeIntegerSemiring() == NN
 False

+but + + +sage: NN == NonNegativeIntegerSemiring() +False +

jdemeyer commented 8 years ago

Description changed:

--- 
+++ 
@@ -8,5 +8,7 @@

sage: NN == NonNegativeIntegerSemiring() -False +True +sage: NonNegativeIntegerSemiring() == NN._get_object() +True

nbruin commented 8 years ago
comment:12

There are many subtle problems with lazy_import and some of them are fairly fundamental. The basic take-away is: don't lazy_import objects, but lazy_import functions that produce objects (NN is problematic but NonNegativeIntegerSemiring() is not).

LazyImport objects try to be as transparent as possible, but obviously won't succeed perfectly. They try to remove themselves once non-trivially accessed (that is, basically when one of their attributes gets accessed), and essentially LazyImport objects should only be used to look up attributes on.

Many of our LazyImport objects get further imported erroneously. For instance, importing a LazyImport object from elsewhere via a from ... import <LazyImportObject> doesn't work properly, because the LazyImport object doesn't get an opportunity to know about the new namespace in which it gets imported. An example is NN as referenced here.

If we insert a little helper function into sage.misc.lazy_import (it has to be there because lazy_import doesn't offer a pxd):

def att(a):
    cdef LazyImport b
    b = a
    return {"_object": b._object,
            "_module": b._module,
            "_name": b._name,
            "_as_name": b._as_name,
            "_namespace": b._namespace,
            "_at_startup": b._at_startup,
            "_deprecation": b._deprecation}

then you can see from

sage: sage.misc.lazy_import.att(NN)['_namespace']['__name__']
'sage.rings.semirings.all'
sage: sage.misc.lazy_import.att(NN)['_name']
'NN'
sage: sage.misc.lazy_import.att(NN)['_as_name']
'NN'

that accessing NN in the toplevel scope will never resolve. On every access, the LazyImport object will dutifully search the sage.rings.semirings.all scope to replace its occurrence there (which only happens the first time of course), but that's not the binding through which it gets accessed afterwards.

If instead you do (and this is how NN should be imported into the toplevel if at all):

sage: lazy_import(sage.misc.lazy_import.att(NN)['_module'], sage.misc.lazy_import.att(NN)['_name'])
sage: type(NN)
<type 'sage.misc.lazy_import.LazyImport'>
sage: NN
Non negative integer semiring
sage: type(NN)
<class 'sage.rings.semirings.non_negative_integer_semiring.NonNegativeIntegerSemiring_with_category'>

As you can see, basically even

from sage.rings.semirings.non_negative_integer_semiring import NN

is an erroneous use of a LazyImport object, because it does not amount to attribute lookup.

The reason why mathematically interesting objects themselves should probably not be lazily imported, but only their access functions is because

sage: { NN : 1 }

will always be problematic: The LazyImport object doesn't get a chance to remove itself.

On the other hand, accessing the object via NonNegativeIntegerSemiring() works nicely.

Of course, NonNegativeIntegerSemiring itself is incorrectly imported.

We could clean up scopes (but that should happen in basically all __all__ files etc.) via something along the lines of:

scope=globals()
for name,value in scope.iteritems():
    if isinstance(value,sage.misc.lazy_import.LazyImport):
        T= sage.misc.lazy_import.att(value)
        if T['_namespace'] is not scope:
            scope[name] = sage.misc.lazy_import.LazyImport(T['_module'],T['_name'],T['_as_name'],scope,T['_at_startup'],T['_deprecation'])

Rather than hobbling EqualityById to accommodate for the hack that is LazyImport, we should avoid erroneous use of LazyImport. The problem with EqualityById is just a symptom of a general problem. There are many tickets about this, see e.g. #12482

jdemeyer commented 8 years ago
comment:13

Replying to @nbruin:

We could clean up scopes (but that should happen in basically all __all__ files etc.)

Of course, now the question becomes: which scopes? If you can hand me a list of dicts which contain all relevant lazy imports, we can indeed do something.

There are many tickets about this, see e.g. #12482

Thanks for the pointer, I fixed this.

jdemeyer commented 8 years ago

Description changed:

--- 
+++ 
@@ -12,3 +12,12 @@
 sage: NonNegativeIntegerSemiring() == NN._get_object()
 True

+ +This gives problems with CachedRepresentation: + + +sage: from sage.misc.lazy_import import LazyImport +sage: lazyZZ = LazyImport('sage.rings.integer_ring', 'ZZ') +sage: PolynomialRing(lazyZZ, 'x') is PolynomialRing(ZZ, 'x') +False +

jdemeyer commented 8 years ago
comment:14

One thing which we could easily do is to change cached_function() and friends to automatically dereference lazy imports. This will fix at least the problem with CachedRepresentation I think.

jdemeyer commented 8 years ago

Changed commit from b21aa6e to none

jdemeyer commented 8 years ago

Changed branch from u/cheuberg/19152-arb-misc-lazy-import to none

vbraun commented 8 years ago
comment:16

Still if we agree that it can't be done safely then maybe it's a bad idea?

An alternative might be to have a proxy object that, instead of trying to look like the original object, relies on the preparser to replace it? E.g. the preparser replaces NN with NonNegativeIntegerSemiring(). And library code would be forced to import from the original location.

jdemeyer commented 8 years ago
comment:18

Replying to @vbraun:

Still if we agree that it can't be done safely then maybe it's a bad idea?

What do the two "it"s refer to in this sentence?

Fixing cached functions can be done safely. We already pre-process arguments to cached functions in src/sage/misc/function_mangling.pyx, so it's not hard to also dereference lazy imports there. Of course this does not fix every single use-case of lazy imports, but this is still an important use-case.

Automatically "fixing" lazy imports in globals()-like dicts can be done, but there the hard problem is to determine which dicts should be processed.

An alternative might be to have a proxy object that, instead of trying to look like the original object, relies on the preparser to replace it? E.g. the preparser replaces NN with NonNegativeIntegerSemiring().

I'm not convinced that we want to involve the preparser for this. What if somebody writes

sage: NN = some_completely_unrelated_object
sage: f(NN)

you don't want this to be replaced by

sage: NonNegativeIntegerSemiring() = some_completely_unrelated_object
sage: f(NonNegativeIntegerSemiring())
nbruin commented 8 years ago
comment:19

Replying to @jdemeyer:

Of course, now the question becomes: which scopes? If you can hand me a list of dicts which contain all relevant lazy imports, we can indeed do something.

All scopes that from ... import a symbol that is a LazyImport. That includes many "all" files. Each of those should basically finish with a

sage.misc.lazy_import.fix_sloppily_constructed_namespace(globals())

(and then we should hope that no fancy shenanigans have happened with the bindings during module initialization)

We should consider how badly this affects startup times. In time-critical bits, it may be better to use a fresh lazy_import command instead instead of fixing the namespace afterwards.

One could run this process on [m for m in sys.modules().itervalues() if m is not None] (there are quite some entries in there that are None. I don't know what they're doing there. Vanilla python has that too), but we'd really be meddling with data that isn't ours, so I'd expect that to lead to more fragileness.

The namespaces registered under sys.modules() would certainly provide a good starting point for assessing the scale of the problem.

vbraun commented 8 years ago
comment:20

Replying to @jdemeyer:

What if somebody writes

sage: NN = some_completely_unrelated_object
sage: f(NN)

That would be perfectly fine. The preparser can't be a regex but has to look at the AST, of course. IPython already has support for that. Its also easier to implement since it leverages the Python parser.

vbraun commented 8 years ago
comment:21

Replying to @jdemeyer:

Still if we agree that it can't be done safely then maybe it's a bad idea?

What do the two "it"s refer to in this sentence?

Both "it" refer to "import of any object for which 'is'-comparison matters". As opposed to factory functions.

nbruin commented 8 years ago
comment:22

OK, and these are the counts for lazy_import objects bound in scopes that is not theirs:

from sage.misc.lazy_import import LazyImport,att #that's the helper function
def misbound_lazies(S):
  return [k for k,v in S.iteritems() if type(v) is LazyImport and att(v)['_namespace'] is not S]
M=[(k,len(misbound_lazies(m.__dict__))) for k,m in sys.modules.iteritems() if m is not None]
sage: [m for m in M if m[1] > 0]
[('sage.combinat.tableau_tuple', 1),
 ('sage.libs.linbox.linbox', 1),
 ('sage.groups.libgap_mixin', 1),
 ('__main__', 190),
 ('sage.categories.all', 1),
 ('sage.combinat.all', 34),
 ('sage.calculus.all', 1),
 ('sage.geometry.all', 3),
 ('sage.functions.piecewise', 1),
 ('sage.structure.sage_object', 1),
 ('sage.combinat.partition_tuple', 1),
 ('sage.modular.arithgroup.congroup_generic', 1),
 ('sage.schemes.all', 6),
 ('sage.rings.all', 3),
 ('sage.functions.special', 1),
 ('sage.functions.airy', 1),
 ('sage.all_cmdline', 190),
 ('sage.libs.pari.gen_py', 4),
 ('sage.structure.coerce', 1),
 ('sage.functions.bessel', 1),
 ('sage.functions.orthogonal_polys', 1),
 ('sage.numerical.interactive_simplex_method', 1),
 ('sage.combinat.integer_vectors_mod_permgroup', 1),
 ('sage.groups.all', 10),
 ('sage.combinat.crystals.tensor_product', 1),
 ('sage.combinat.composition', 1),
 ('sage.categories.groups', 1),
 ('sage.combinat.partition', 1),
 ('sage.all', 165)]
sage: len(set(flatten([misbound_lazies(m.__dict__) for m in sys.modules.values() if m is not None])))
195

So, assuming that symbol name is a good indication, there seem to be about 195 distinct lazy_import objects around that are bound in wrong scopes.

Most frequently occurring:

sage: L=[misbound_lazies(m.__dict__) for m in sys.modules.values() if m is not None]
sage: sorted(Counter(flatten(L)).items(),key=lambda a:a[1])[-20:]
[('WehlerK3Surface', 4),
 ('ClusterQuiver', 4),
 ('CrystalOfNakajimaMonomials', 4),
 ('KostkaFoulkesPolynomial', 4),
 ('polytopes', 4),
 ('ExtendedAffineWeylGroup', 4),
 ('QuaternionMatrixGroupGF3', 4),
 ('KirillovReshetikhinTableaux', 4),
 ('CrystalOfGeneralizedYoungWalls', 4),
 ('maxima_calculus', 4),
 ('AffineCrystalFromClassical', 4),
 ('ClusterSeed', 4),
 ('QuiverMutationType', 4),
 ('maxima', 4),
 ('NonNegativeIntegerSemiring', 5),
 ('Polyhedron', 5),
 ('MatrixGroup', 5),
 ('AsymptoticRing', 5),
 ('cartesian_product', 7),
 ('NN', 9)]
jdemeyer commented 8 years ago
comment:23

Replying to @vbraun:

Both "it" refer to "import of any object for which 'is'-comparison matters". As opposed to factory functions.

Now the question becomes: to what extent does equality matter? One of my proposals is to fix lazy imports in cached functions. That would seriously reduce the extent to which equality matters. It would certainly fix this:

sage: from sage.misc.lazy_import import LazyImport
sage: lazyZZ = LazyImport('sage.rings.integer_ring', 'ZZ')
sage: PolynomialRing(lazyZZ, 'x') is PolynomialRing(ZZ, 'x')
False

What do you think about this?

nbruin commented 8 years ago
comment:24

Replying to @jdemeyer:

Now the question becomes: to what extent does equality matter? One of my proposals is to fix lazy imports in cached functions. That would seriously reduce the extent to which equality matters. It would certainly fix this:

sage: from sage.misc.lazy_import import LazyImport
sage: lazyZZ = LazyImport('sage.rings.integer_ring', 'ZZ')
sage: PolynomialRing(lazyZZ, 'x') is PolynomialRing(ZZ, 'x')
False

What do you think about this?

I know that Robert Bradshaw did not endorse the use of LazyImport for imports like this. The intended usage scenario was really just to replace "import" with a few extensions to support "from .. import .." for functions/classes/submodules. He was fully aware that the shim that LazyImport objects provide is imperfect. In his opinion, you shouldn't import ZZ this way. You can lazy-import sage.rings.integer_ring as integer_ring and then address ZZ as integer_ring.ZZ.

You can of course try to extend the use of lazy_import and see if you can get it to work for things like NN as well. However, you'll find (as some examples have shown) that it's always fundamentally problematic. I think we're better off rooting out the "bad use" than trying to (imperfectly) support it. It would be nice to support lazy importing like this fully, but I think it would require hacking python to an uncomfortable degree.

The problem isn't huge: I think NN is the worst symbol like this. Perhaps we can just not lazily import it? or otherwise make it accessible via an accessor function or via a lazily imported namespace (rather than importing NN lazily itself).

For a pretty complete tally: (so we're talking about 291 objects)

sage: def lazy_names(S):
....:      return [k for k,v in S.iteritems() if type(v) is LazyImport]
sage: L=[lazy_names(m.__dict__) for m in sys.modules.values() if m is not None]
sage: len(Counter(flatten(L)))
291

There are a lot of interesting bindings there, though:

sage: type(sage.misc.misc.SAGE_ROOT)
<type 'sage.misc.lazy_import.LazyImport'>

To see the types of these lazy import objects:

from sage.misc.lazy_import import LazyImport,att
from collections import Counter
def lazy_objects(S):
  return [v for k,v in S.iteritems() if type(v) is LazyImport]

L=[lazy_objects(m.__dict__) for m in sys.modules.values() if m is not None]
O=[]
for l in L: O.extend(l)
%cpaste
for o in O: #try to load all objects
  try:
    _=repr(o)
  except:
    pass
--
Counter([type(att(o)["_object"]) for o in O])

Which gives:

sage: list(Counter([type(att(o)["_object"]) for o in O]))
[<class 'sage.rings.semirings.non_negative_integer_semiring.NonNegativeIntegerSemiring_with_category'>,
 <type 'sage.misc.inherit_comparison.InheritComparisonMetaclass'>,
 <type 'sage.misc.lazy_import.LazyImport'>,
 <class 'sage.combinat.cluster_algebra_quiver.quiver_mutation_type.QuiverMutationTypeFactory'>,
 <type 'module'>,
 <class 'sage.modular.arithgroup.congroup_sl2z.SL2Z_class_with_category'>,
 <class 'sage.categories.cartesian_product.CartesianProductFunctor'>,
 <type 'dict'>,
 <class 'sage.combinat.finite_state_machine_generators.AutomatonGenerators'>,
 <class 'sage.geometry.hyperplane_arrangement.library.HyperplaneArrangementLibrary'>,
 <type 'instance'>,
 <class 'sage.combinat.finite_state_machine_generators.TransducerGenerators'>,
 <class 'sage.libs.gap.libgap.Gap'>,
 <type 'bool'>,
 <type 'str'>,
 <class 'sage.dev.sagedev_wrapper.SageDevWrapper'>,
 <type 'NoneType'>,
 <type 'sage.misc.classcall_metaclass.ClasscallMetaclass'>,
 <class 'sage.interfaces.genus2reduction.Genus2reduction'>,
 <type 'function'>,
 <class 'sage.interfaces.maxima_lib.MaximaLib'>,
 <type 'builtin_function_or_method'>,
 <type 'list'>,
 <type 'type'>,
 <class 'sage.databases.findstat.FindStat'>,
 <class 'sage.rings.invariant_theory.InvariantTheoryFactory'>,
 <class 'sage.misc.inherit_comparison.InheritComparisonClasscallMetaclass'>,
 <class 'abc.ABCMeta'>]

Most of these objects are straightforward callables, that are unlikely to be used as objects to be worked with. The remaining objects could be investigated if they need to by lazily imported by themselves or whether some surrounding scope can be lazily imported instead.

jdemeyer commented 8 years ago
comment:25

Replying to @nbruin:

Replying to @jdemeyer:

Now the question becomes: to what extent does equality matter? One of my proposals is to fix lazy imports in cached functions. That would seriously reduce the extent to which equality matters. It would certainly fix this:

sage: from sage.misc.lazy_import import LazyImport
sage: lazyZZ = LazyImport('sage.rings.integer_ring', 'ZZ')
sage: PolynomialRing(lazyZZ, 'x') is PolynomialRing(ZZ, 'x')
False

What do you think about this?

I know that Robert Bradshaw did not endorse the use of LazyImport for imports like this.

Sure, it was just to give some kind of "worst case" example, with no namespace binding at all.

jdemeyer commented 8 years ago
comment:26

Replying to @nbruin:

You can of course try to extend the use of lazy_import and see if you can get it to work for things like NN as well.

I would certainly like to try that. It would be a pity if rings (like NN) are fundamentally incompatible with lazy_import. Consider arb for example: it makes sense to define a global RBF (for real ball field), analogous to RR. It also makes a lot of sense to lazily import this: it's unlikely to be commonly used, it would save an import of at least 2 Python modules and it would save the loading of the external library libarb.

nbruin commented 8 years ago
comment:27

Replying to @jdemeyer:

Replying to @nbruin:

You can of course try to extend the use of lazy_import and see if you can get it to work for things like NN as well.

I would certainly like to try that. It would be a pity if rings (like NN) are fundamentally incompatible with lazy_import. Consider arb for example: it makes sense to define a global RBF (for real ball field), analogous to RR. It also makes a lot of sense to lazily import this: it's unlikely to be commonly used, it would save an import of at least 2 Python modules and it would save the loading of the external library libarb.

We can do that at the cost of a namespace:

realballfield.RBF
natural_numbers.NN

(where realballfield and natural_numbers can be lazily imported) or at the cost of an accessor function (lazily imported):

RealBallField()
NaturalNumbers()

The fact that

D=dict()
D[NN]=1
D[NN]=2

will never work properly makes trying to pretend otherwise a non-starter in my opinion. You'd have to reach too deeply into python (you probably wouldn't even be able to distinguish between the "initialization" code where the Lazy object needs to be passed and genuine access, where you do need to dereference).

We could reach into various sage infrastructures and special-case LazyImports there, but in my opinion that would just increase the surprise factor when other constructs fail.

jdemeyer commented 8 years ago
comment:28

Replying to @nbruin:

You'd have to reach too deeply into python

Are you thinking of ma_lookup()? That could work, but I agree that it would be a major hack.

nbruin commented 8 years ago
comment:29

Replying to @jdemeyer:

Are you thinking of ma_lookup()? That could work, but I agree that it would be a major hack.

That would be one hook. But there's more.

sage: lazy_import("sage.rings.integer_ring","ZZ")
sage: id(ZZ)
140457737914168
sage: ZZ
Integer Ring
sage: id(ZZ)
140458433423744

Note that the user didn't rebind ZZ and yet its identity has changed. That can affect all kinds of things. You'd basically need LazyImport to be transparent for being put into argument lists as well. And that's just horrible (and undebuggable!). I think it would be a major maintainability penalty to try and support that kind of stuff, for relatively little gain. I would say the minor inconvenience for users that objects supported by code that is lazily imported cannot be available straight away in the top namespace is preferable.

But perhaps your experiments end up indicating something else.

jdemeyer commented 8 years ago
comment:30

Replying to @nbruin:

I would say the minor inconvenience for users that objects supported by code that is lazily imported cannot be available straight away in the top namespace is preferable.

So, what's your proposal then? Close this ticket as wontfix and change the lazy_import of NN (and similar objects) to a "real" import?

But perhaps your experiments end up indicating something else.

Which "experiments" do you mean? You mean with ma_lookup()?

nbruin commented 8 years ago
comment:31

Replying to @jdemeyer:

So, what's your proposal then? Close this ticket as wontfix and change the lazy_import of NN (and similar objects) to a "real" import?

Yes. Or if NN is too expensive to import at startup, adapt the places where NN is accessed (the count above shows about 10 modules (including __main__ and I think some double counting) that reference it via a lazy import) in a way that triggers the import as soon as NN is accessed (i.e., either by accessing NN through a namespace that's lazily imported or via an accessor function). I don't see a reasonable alternative while maintaining a semantics model that at least resembles that of python.

The other thing we should do is clean up the mess of mis-lazy_imported objects. As the list above shows, a lot of lazy_imports don't have a chance of ever clearing themselves up. The penalty of accessing an object through a lazy_import isn't that high, but the problem is these things compound. We already have lazy_imports that point to lazy_imports.

I propose:

but perhaps that's better handled on a different ticket.

jdemeyer commented 7 years ago

Branch: u/jdemeyer/lazy_import_breaks_cachedrepresentation

jdemeyer commented 7 years ago

Commit: d501601

jdemeyer commented 7 years ago

New commits:

d501601WithEqualityById.__richcmp__: bail out if "other" is no instance of WithEqualityById
jdemeyer commented 7 years ago

Author: Jeroen Demeyer

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

2f5aeadWithEqualityById.__richcmp__: bail out if "other" is no instance of WithEqualityById
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

Changed commit from d501601 to 2f5aead

jdemeyer commented 7 years ago

Dependencies: #23102

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

Changed commit from 2f5aead to 759da06

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

256bf19Move richcmp stuff to new file richcmp.pyx
b591b78Support `__richcmp__` in Python classes
47f97ecImplement RealSet comparisons using __richcmp__
acfd0aaTypo
759da06WithEqualityById.__richcmp__: bail out if "other" is no instance of WithEqualityById
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

Changed commit from 759da06 to 866eddd

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

49b6f76Support `__richcmp__` in Python classes
d343d9dImplement RealSet comparisons using __richcmp__
3aac3ccTypo
86e9fcfInstall slot wrappers for rich comparison methods
4d1021bFurther fixes and tests for richcmp_method
866edddWithEqualityById.__richcmp__: bail out if "other" is no instance of WithEqualityById
embray commented 6 years ago
comment:41

I haven't read all the comments on this ticket, but the problem statement is clear and the proposed fix makes perfect sense.

embray commented 6 years ago

Reviewer: Erik Bray

vbraun commented 6 years ago

Changed branch from u/jdemeyer/lazy_import_breaks_cachedrepresentation to 866eddd

fchapoton commented 1 year ago

Changed commit from 866eddd to none