On Mon, May 30, 2016 at 6:42 PM, William Stein <wst...@gmail.com> wrote: > On Mon, May 30, 2016 at 9:22 AM, Erik Bray <erik.m.b...@gmail.com> wrote: >> Hi all, >> >> I recently needed to dive into the sage_setup.autogen.interpreters >> module in order to make some small changes. > > Quick link: > > https://github.com/sagemath/sage/blob/master/src/sage_setup/autogen/interpreters.py > > It's the fast_callable stuff. > > Were you diving in to make (sage expression)(numpy array) work? If so, > awesome!
Sadly, not this time. It's to make the generated code build on Windows. However, I also want to work on the issue you mentioned, and cleaning up this code a bit will likely help. >> The file is over 4000 >> lines long, which is a bit on the long side for your typical Python >> file, though not egregious by any means. That said, when trying to >> understand some relatively complicated code I find it helpful to break >> up into smaller bite-sized logical chunks that are easy to get around >> in an editor and reason about. When and how to do this can of course >> be highly subjective. >> >> In the case of autogen.interpreters, in the process of understanding >> it, it was my immediate instinct, perhaps a bit impulsive, to start >> breaking it into multiple files anyways, and about half an hour later >> I've done so with success. >> >> I think it would be a good change to feed back into sage, but it's >> also a bit frivolous since there are no other substantive changes. I >> think it makes the code easier to understand. But of course the main >> downside to this kind of refactoring is that it makes the history >> harder to follow--not impossible--just harder. >> >> How does this community feel about this sort of refactoring? On the >> outset it could be seen as frivolous, but in the long term it can be >> for the best, especially as development continues and some of the >> resulting modules grow larger on their own. > > +1 -- my only concern is for code that does something like: > > "from sage_setup.autogen.interpreters import blah" > > Maybe this isn't an issue in this case, but in general it could be. Yep. I made sure that it still fills out the same namespace more or less (as far as anything seems to care about). > Sage devs are very good at git, so dealing with the history issue > isn't at all a show stopper. I've seen people complain about this before which is partly why I ask. I think it's a valid complaint too, but definitely not impossible to overcome. > Separating code into many smaller modules really does provide a lot of > genuine value, in that it makes scope and dependencies much, much > clearer, which makes the code potentially way easier to understand. Yep, I agree completely. Thanks, Erik -- You received this message because you are subscribed to the Google Groups "sage-devel" group. To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+unsubscr...@googlegroups.com. To post to this group, send email to sage-devel@googlegroups.com. Visit this group at https://groups.google.com/group/sage-devel. For more options, visit https://groups.google.com/d/optout.
