#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.