#10963: More functorial constructions
-------------------------------------+-------------------------------------
       Reporter:  nthiery            |        Owner:  stumpc5
           Type:  enhancement        |       Status:  needs_work
       Priority:  major              |    Milestone:  sage-6.1
      Component:  categories         |   Resolution:
       Keywords:  days54             |    Merged in:
        Authors:  Nicolas M. Thiéry  |    Reviewers:  Simon King, Frédéric
Report Upstream:  N/A                |  Chapoton
         Branch:                     |  Work issues:  Detect and fix
  public/ticket/10963                |  Heisenbugs
   Dependencies:  #11224, #8327,     |       Commit:
  #10193, #12895, #14516, #14722,    |  8eaf51a82c4e2194769db13457979ae601ebbc04
  #13589, #14471, #15069, #15094,    |     Stopgaps:
  #11688, #13394, #15150, #15506     |
-------------------------------------+-------------------------------------

Comment (by nbruin):

 I am afraid that Simon's solution, while directly addressing the apparent
 issue, is insufficient. You are using a caching mechanism--something that
 should be used as a performance tool--to avoid an infinite recursion.
 That's bad. You're putting program logic in a place where people won't
 expect it.

 It also relies on the cache being of a particular form and in a particular
 place. That's an implementation detail of our cached function decorator.
 It's already hard to figure out where that cache is stored. I can easily
 see the location changing in the future. Then you'll get stuff cached in
 two places or some hard-to-debug error.

 Another issue (and this is already in the code) is that
 `base_category_and_axiom` is a module level function, so has a global
 cache. Any input to it (including erroneous input?) will be stored for
 eternity: memory leak.

 In addition, `_base_category_and_axiom` is a lazy class attribute, so it
 has a cache of its own. Why are we caching things twice?

 Finally, and there are some design decisions in the ticket itself:
 Currently, there is code that relies on packages being in
 `sage.categories` by looking at the `__name__` and doing string mangling
 on it. Furthermore, it relates the class name `CamelCase` to the module
 `camel_case`. These are all fine a programming ''conventions'' but I find
 it questionable if it's a good idea to engrave them in ''program logic''.
 This is what I referred to when I wrote "doesn't read like python" above.
 If this were happening in a specialized little corner it wouldn't be so
 alarming, but categories are supposed to be a fundamental tool to govern
 inheritance in sage and hence part of the infrastructure. That means a lot
 of people will have to touch and maintain it in the future, so such code
 should really be held to a higher standard in terms of understandability,
 cleanness, and design.

 Writing a document that explains how to work with it and maintain the code
 might help, but isn't enough: documents tend to get out of sync with code
 over time (even docstrings!). But at least it would mean we have a record
 of the original author's intent.

--
Ticket URL: <http://trac.sagemath.org/ticket/10963#comment:260>
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/groups/opt_out.

Reply via email to