Opened 17 months ago

Closed 17 months ago

#32112 closed defect (fixed)

Bug in Chart.__init__ regarding the determination of top charts

Reported by: Eric Gourgoulhon Owned by:
Priority: major Milestone: sage-9.4
Component: manifolds Keywords: coordinate chart
Cc: Matthias Köppe, Travis Scrimshaw, Michael Jung, Volker Braun Merged in:
Authors: Eric Gourgoulhon Reviewers: Matthias Koeppe
Report Upstream: N/A Work issues:
Branch: public/manifolds/top_charts_32112 (Commits, GitHub, GitLab) Commit: 4e316e9f4e3e714192568594197afb34cfd8121f
Dependencies: Stopgaps:

Status badges

Description

As noticed in ​https://trac.sagemath.org/ticket/31901#comment:28, there is an issue when constructing two charts sharing the same coordinate symbols but not being otherwise related: 

sage: M = Manifold(2, 'M', structure='topological')
sage: U = M.open_subset('U')
sage: V = M.open_subset('V')
sage: XU = U.chart('x y')
sage: XV = V.chart('x y')
sage: M.top_charts()
[Chart (U, (x, y))]

The chart XV should also appear as a top chart on M.

Change History (8)

comment:1 Changed 17 months ago by Eric Gourgoulhon

Authors: → Eric Gourgoulhon
Branch: → public/manifolds/top_charts_32112
Cc: Matthias Köppe Travis Scrimshaw Michael Jung added
Commit: → 4e316e9f4e3e714192568594197afb34cfd8121f
Status: new → needs_review

New commits:

​4e316e9Fix bug in Chart.__init__ regarding the determination of top charts (Trac #32112)

comment:2 Changed 17 months ago by Matthias Köppe

Reviewers: → Matthias Koeppe
Status: needs_review → positive_review

Thanks! This is working well.

comment:3 Changed 17 months ago by Matthias Köppe

It would have been good to do this on top of #32009 though

comment:4 in reply to:  3 Changed 17 months ago by Eric Gourgoulhon

Thanks for the review.

Replying to mkoeppe:

It would have been good to do this on top of #32009 though

Could you explain why? I mean, why _domain should become a Cython attribute?

comment:5 Changed 17 months ago by Matthias Köppe

In #31894 I hope to make Chart a subclass of sage.categories.map.Map so that all functionalities of morphisms such as composition etc. become available. Map is a Cython class in which _domain happens to be a Cython attribute.

comment:6 Changed 17 months ago by Matthias Köppe

As #32009 came back with a bug, I have made the present ticket a dependency of #32009 instead.

comment:7 in reply to:  5 Changed 17 months ago by Eric Gourgoulhon

Replying to mkoeppe:

In #31894 I hope to make Chart a subclass of sage.categories.map.Map so that all functionalities of morphisms such as composition etc. become available. Map is a Cython class in which _domain happens to be a Cython attribute.

Thanks for the explanation.

comment:8 Changed 17 months ago by Matthias Köppe

Cc: Volker Braun added
Resolution: → fixed
Status: positive_review → closed

Apparently this has been merged as part of #32089.

Note: See TracTickets for help on using tickets.