#18529: Topological manifolds: basics
-------------------------------------+-------------------------------------
       Reporter:  egourgoulhon       |        Owner:  egourgoulhon
           Type:  enhancement        |       Status:  needs_review
       Priority:  major              |    Milestone:  sage-6.10
      Component:  geometry           |   Resolution:
       Keywords:  topological        |    Merged in:
  manifolds                          |    Reviewers:
        Authors:  Eric Gourgoulhon   |  Work issues:
Report Upstream:  N/A                |       Commit:
         Branch:                     |  6dec6d592a09e56921b9a761827309dd31ae2533
  public/manifolds/top_manif_basics  |     Stopgaps:
   Dependencies:  #18175             |
-------------------------------------+-------------------------------------

Comment (by tscrim):

 Some things from looking over the current structure:

 - I would separate out parts of the subset class that applies to
 `Top(ological)Manifold` and `TopManifoldSubset` into an ABC (abstract base
 class) so you don't have to do things like `self is manifold`.

 - Maybe put the subsets into the `Subobjects` class and maybe consider not
 making it a facade?

 - Should manifolds and their subsets be `UniqueRepresentation`? I
 understand that the name as the only input means they are essentially
 treated as variables, but it means you have to do things like
 `TopManifold._clear_cache_()` (which may be needed not just necessarily in
 doctests if one does this before the gc comes around).

 - Related to the above, we want parents to be hashable and have a good
 equality. I know that by making it a `UniqueRepresentation`, many of these
 issues are solved. However the manifolds are essentially mutable, but by
 doing a `__hash__` which only depends upon the name(s) and dimension means
 this is okay (`__eq__` defaults to check by `is`). This would alleviate
 the need for `UniqueRepresentation`.

 - In the tests for `_element_constructor_`, you shouldn't call it
 explicitly but via `X(...)`, which also checks that it is working
 correctly with the coercion framework.

 - I don't quite agree with checking `self._field == RR` for real charts as
 the precision should not matter. I would check `isinstance(self._field,
 RealField)` instead. Granted, we probably should have a function that
 checks against all known real field implementations...

 - In a similar vein, I don't like the input of `'real'` to `RR` (and
 `'complex'` to `CC`). I would make the user be explicit about what they
 want unless you are doing to do some special handling to make this pass
 special arguments to an underlying `SR` implementation.

--
Ticket URL: <http://trac.sagemath.org/ticket/18529#comment:27>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica, 
and MATLAB

-- 
You received this message because you are subscribed to the Google Groups 
"sage-trac" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/d/optout.

Reply via email to