Changes between Initial Version and Version 1 of Ticket #13215, comment 67


Ignore:
Timestamp:
07/21/16 02:38:51 (5 years ago)
Author:
tscrim
Comment:

Legend:

Unmodified
Added
Removed
Modified
  • Ticket #13215, comment 67

    initial v1  
    77That must of gotten lost when I had connectivity issues. This was suppose to be "by using a consistent API".
    88
    9 > > > - There's an option `sparse=False` which, if set to `True`, returns a `NotImplementedError`.
    10 > > >  I suggest to remove it for now in order to not confuse the user, and reinstate it later
    11 > > >  when it will be fully implemented. In this case, also remove all references to this feature
    12 > > >  in the documentation.
    13 > > > - As I suggest to remove some not implemented but planned features, it might be a good
    14 > > >  idea to add a `TODO` block as a reminder of these features (sparse rings, multivariate rings).
    15 > >
    16 > > I disagree with these comments. This is documenting an upcoming feature, it helps keeps the API consistent with future plans and other parts of Sage, and tells someone reading the code what is currently not implemented.
    17 >
    18 > This is debatable: sparse skew polynomials are not very useful whenever there is a derivation (since sparsity is not retained well across operations), and it's not very clear what a multivariate skew ring is. The most natural generalisation of this structure is Ore polynomials, but that would likely be a separate class. I don't think it makes sense to leave implementation artifacts that will, most likely, never lead to real implementations. In the odd case that they *would* lead to implementations, we could reinstate them with no penalty.
    19 >
    209> > Also, from a cursory glance, there seems to be a lot of duplication with the current implementations of polynomials (i.e., the elements). I feel there could be a lot of simplification of the code by either subclassing or creating a mix-in (cython) class that overrides the `_mul_` of polynomials. Is there a reason why you didn't do this? This might entail doing some refactoring/abstracting of the polynomial code though, but it should make the implementation of skew polynomials a lot easier (and bug proof).
    2110>