And a big thank you to you for submitting it in the first place! Aaron Meurer
On Jun 29, 2012, at 3:05 AM, Tom Bachmann <e_mc...@web.de> wrote: > Heya, > > my biggest pull request got green light and was merged a few minutes ago. A > very big *thank you* to everyone (in particular aaron) who reviewed this huge > pile of code. > > Tom > > On 10.06.2012 10:34, Tom Bachmann wrote: >> Hi, >> >> first of all I'd like to apologize for being slightly unresponsive >> during the last couple of weeks ... it's been exam time. >> >> Partly as a result of this, my pull requests have been sitting around >> for a rather long time now. As a reminder, there is a trigsimp extension >> [1], and various commutative algebra related pull requests [2]. As far >> as I am aware, these pull requests are fully rebased and pass tests in >> all python versions from 2.5 to 3.2. This applies in particular to the >> trigsimp PR - could anyone give this a final once-over and decide if it >> can be committed? >> >> Regarding the AGCA pull requests, I have to (embarrasedly) say that >> while I hope the many smallish PRs made reviewing easier, I have >> somewhat lost track of which of these are commit-ready and which not. As >> far as I can tell I have addressed all comments, with exception of those >> elaborated on below: >> >> - naming of the implementation classes >> >> In my system, there are "abstract" base classes implementing user-facing >> functionality, and "concrete" subclasses supplying vital methods used by >> the abstract code. For example, there is an "abstract" class for >> submodules (called SubModule), which implements all the niceties, and >> there is a "concrete" subclass which implements core functionality, like >> element membership and syzygy computation. It is currently named >> SubModulePolyRing. This is because while the concept of submodules makes >> sense over arbitrary rings, this implementation uses groebner basis >> computations, which only work for polynomial rings. >> >> In fact, the implementation only works with polynomial rings over >> *fields*. It has been suggested that if (or when) this code might be >> extended to work in more general cases (there certainly are groebner >> basis algorithms for various more general cases), the name >> "FreeModulePolyRing" might turn out to be too general - i.e. what should >> the new class be called? >> >> I see I'm not managing to explain this simple issue in a concise way. So >> let me put it like this: what should the naming convention be for the >> concrete subclasses? Should I rename SubModulePolyRing to >> SubModulePolyRingField, or perhaps SubModulePolyField? Or might >> SubModulePolyRing do, and the new class might be called SubModulePolyPID? >> >> - printing of matrices >> >> As discussed in issue 2865, matrix 'str' printing should not produce 2D >> pictures. The same goes for my homomorphism class. However the >> homomorphism class actually *uses* the matrix printing. So my question >> is: is anyone working on fixing the matrix printing problem? Because if >> not I will do it (and then my homomorphism printing will be fixed >> automatically - except for a few doctests it will break). >> >> - coercion in equality testing >> >> This is a bit of a conceptual issue. In the module classes, I'm allowing >> expressions like >> >> module_element == [x, y, z]. >> >> Here the __eq__ function of module_element automatically tries to coerce >> [x, y, z]. Functions like __add__ etc also do this. I found it handy, >> because otherwise one always has to write >> >> module_element = my_module.convert([x, y, z]). >> >> I guess my general question is: is this a good idea? Does the gained >> syntactic flexiblity outweigh the risks? (what risks/problems are >> there?) A related issue is that I implemented similar automatic coercion >> in the new QuotientRing class. Again, I think this is quite handy, since >> it allows writing >> >> foo == 1 >> >> instead of writing "foo = my_ring.convert(1)" or "foo == 1 + my_ideal". >> On the other hand the last notation can be fairly short if used >> consistently. The problem with this automatic coercion in QuotientRing >> is that no other polys/domains classes do it, so it's somewhat >> inconsistent. >> >> >> Ok. I think I have woffled for long enough. Please let me know what you >> think. >> >> Best, >> Tom >> >> [1] https://github.com/sympy/sympy/pull/1258 >> [2] https://github.com/sympy/sympy/pull/1209 >> https://github.com/sympy/sympy/pull/1218 >> etc >> > > -- > You received this message because you are subscribed to the Google Groups > "sympy" group. > To post to this group, send email to sympy@googlegroups.com. > To unsubscribe from this group, send email to > sympy+unsubscr...@googlegroups.com. > For more options, visit this group at > http://groups.google.com/group/sympy?hl=en. > -- You received this message because you are subscribed to the Google Groups "sympy" group. To post to this group, send email to sympy@googlegroups.com. To unsubscribe from this group, send email to sympy+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/sympy?hl=en.
