Subintervals of OpenInterval and UniqueRepresentation

[mjungmath]

At the moment, we have the following behavior:

sage: I = OpenInterval(0,2)
sage: J = OpenInterval(0,1, ambient_interval=I, coordinate='t')
sage: I.open_interval(0,1)
Traceback (most recent call last)
...
ValueError: the name '(0, 1)' is already used for another subset of the Real interval (0, 2)

Even though the use of OpenInterval(0,1, ambient_interval=I) is not intended, this is still a blind spot.

The reason for this behavior comes from the UniqueRepresentation and how the subintervals are constructed.

I propose a fix using __classcall_private__.

Depends on #30799

CC: @egourgoulhon @tscrim @tobiasdiez

Component: manifolds

Author: Michael Jung

Branch/Commit: 200942c

Reviewer: Travis Scrimshaw

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

[mjungmath]

Branch: u/gh-mjungmath/subintervals_of_openinterval_and_uniquerepresentation

[mjungmath]

Author: Michael Jung

[mjungmath]

comment:3

Ready for review.

---

New commits:

25e96e9

Trac #30830: add `__classcall_private__` to OpenInterval

[mjungmath]

Commit: 25e96e9

[tscrim]

comment:4

You have a doctest failure in continuous_map (see the patchbot). Otherwise LGTM.

[tscrim]

comment:5

Also TEST: -> TESTS::.

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

aab0f14

Trac #30830: utilize UniqueRepresentation instead

[sagetrac-git]

Changed commit from 25e96e9 to aab0f14

[mjungmath]

comment:7

I think, this approach is slighly better because it is closer to the original behavior. Now, the test in continuous_map.py should pass.

[mkoeppe]

comment:8

Side remark: It would be nice to connect OpenInterval also to RealSet (not on this ticket)...

[mjungmath]

comment:9

Replying to @mkoeppe:

Side remark: It would be nice to connect OpenInterval also to RealSet (not on this ticket)...

Nice! Would you take the honor to open the corresponding ticket?

[mkoeppe]

comment:10

That's now #30832

[mjungmath]

comment:11

Replying to @mkoeppe:

That's now #30832

+1

[tscrim]

comment:12

With this change, you can create two distinct intervals with the same (latex) name by passing the resulting values if None is given (e.g., name='R'). While this is me being deliberately evil, it is something I want you to consider and if you are okay with this possible situation happening.

[mjungmath]

comment:13

You mean

sage: R = RealLine(); R                                                         
Real number line R
sage: R1 = RealLine(name='R'); R1                                               
Real number line R
sage: R is R1                                                                   
False

Right?

[tscrim]

comment:14

Replying to @mjungmath:

You mean

sage: R = RealLine(); R                                                         
Real number line R
sage: R1 = RealLine(name='R'); R1                                               
Real number line R
sage: R is R1                                                                   
False

Right?

Yes, exactly.

[mjungmath]

Dependencies: #30799

[mjungmath]

comment:15

This should do, but it doesn't. Adding __classcall__ to RealLine doesn't help either.

[sagetrac-git]

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

8d2f44d	Trac #30799: collect manifolds examples in 'examples' + lazy_import for catalog
224ab51	Trac #30830: add `__classcall_private__` to OpenInterval

[sagetrac-git]

Changed commit from aab0f14 to 224ab51

[sagetrac-git]

Changed commit from 224ab51 to 200942c

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

200942c

Trac #30830: same name yields same instance

[mjungmath]

comment:18

This works now.

I think that OpenInterval should silently return a RealLine instance if lower == minus_infinity and upper == infinity. As Eric pointed out, the Real line is a fully determined mathematical object, and the behaviour here should reflect that.

But this should be part of another ticket.

[tscrim]

comment:19

Thanks. LGTM.

[tscrim]

Reviewer: Travis Scrimshaw

[mjungmath]

comment:20

Thanks for the review.

[vbraun]

Changed branch from u/gh-mjungmath/subintervals_of_openinterval_and_uniquerepresentation to 200942c