#19259: subrings of the symbolic ring
-------------------------------------+-------------------------------------
       Reporter:  dkrenn             |        Owner:
           Type:  enhancement        |       Status:  needs_review
       Priority:  major              |    Milestone:  sage-6.9
      Component:  symbolics          |   Resolution:
       Keywords:                     |    Merged in:
        Authors:  Daniel Krenn       |    Reviewers:
Report Upstream:  N/A                |  Work issues:
         Branch:  u/dkrenn           |       Commit:
  /symbolic-subring                  |  581f3157f57c8b38390cf515e065cf62eacec306
   Dependencies:                     |     Stopgaps:
-------------------------------------+-------------------------------------

Comment (by dkrenn):

 Replying to [comment:15 behackl]:
 > Hello! I started reviewing this ticket, and for now, I have the
 following comments:
 >
 > - `is_variable_valid` --> `has_valid_variable` or `allows_variable`?
 This would be a semantic improvement as the current version does not
 really fit to the other `parent.is_*`-methods, IMHO.

 I agree; changed.

 > - While it is OK that the subring factory checks that every element in
 the `vars`-tuple can be thrown into `SR` in
 `SymbolicSubringFactory.create_key_and_extra_args`, the elements are not
 checked for being valid identifiers. I'd suggest to do so either in the
 factory, or directly in `GenericSymbolicSubring.__init__`.

 Added check.

 > Otherwise, this is possible:
 > {{{
 > sage: SR.subring(accepting_variables=(0, pi, sqrt(2), I))
 > Symbolic Subring accepting the variables 0, I, pi, sqrt(2)
 > }}}

 Doctest added.

--
Ticket URL: <http://trac.sagemath.org/ticket/19259#comment:17>
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 https://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/d/optout.

Reply via email to