#378: User-specified path for load and attach
------------------------------+---------------------------------------------
   Reporter:  was             |       Owner:  was         
       Type:  enhancement     |      Status:  needs_review
   Priority:  major           |   Milestone:  sage-4.6.1  
  Component:  user interface  |    Keywords:              
     Author:  Mitesh Patel    |    Upstream:  N/A         
   Reviewer:                  |      Merged:              
Work_issues:                  |  
------------------------------+---------------------------------------------

Comment(by jsrn):

 Replying to [comment:16 flawrence]:
 > Replying to [comment:10 timdumol]:
 > > Things work perfectly, but I think it might be better to follow the
 Python stdlib in exposing the array itself, instead of covering it in a
 function (`sys.path` instead of `sys.path()`, so similarly,
 `load_attach_path` instead of `load_attach_path()`). Also, it might be
 handy to `os.path.expand_user` any paths handed to the function, so you
 could have load_attach_path.append("~/foo"). What do you think?
 >
 > I've had a bit of a hack at exposing the array itself, but there are
 problems making it sufficiently global.  `sys.path` works for the stdlib
 because `path` is a variable in the `sys` module.  The logical thing to do
 here is to create a `load_attach_path` variable in the preparser module,
 but following the stdlib analogy this would require the user to edit
 `sage.misc.preparser.load_attach_path`, which is not very user friendly.
 >
 > Failing this, it looks like the best you can do is a `from
 sage.misc.preparser import load_attach_path`, but this is a fragile state
 of affairs: if you reassign `load_attach_path` anywhere, either from
 sage's command line or by calling some function like
 `sage.misc.preparser.reset_load_attach_path()`, you end up with a
 situation where `load_attach_path` and
 `sage.misc.preparser.load_attach_path` are different objects.
 >
 > This requires caution on the part of the user and the programmer.  For
 example, `reset_load_attach_path()` can't include
 > {{{
 > load_attach_path = ['.']
 > }}}
 >
 > instead it has to do
 > {{{
 > while load_attach_path != []:
 >     load_attach_path.pop()
 > load_attach_path.append('.')
 > }}}
 >
 > If the user accidentally reassigns `load_attach_path` in the command
 line, they'll have to re-run `from sage.misc.preparser import
 load_attach_path`.  I don't think that this step can be included in a
 `reset_load_attach_path()` function due to the different scopes.
 >
 > So it's possible to expose the `load_attach_path` array to the user, and
 I've written most of a patch that does this, but there's lots of things
 that could go wrong so I'd suggest sticking to the existing patch's
 approach of hiding behind a function.  If others think that the pros
 outweigh the cons then I'll tidy up the patch and upload it.
 >
 > Thoughts?

 Very astute! I was almost finished with my own patch, falling into all the
 same holes as you describe :-) Your concern at least begs the question on
 whether this is what we want to do, as it might fool some users. However,
 a possible usage pattern - in case load_attach_path is fully exposed - is
 the following

 {{{
    import sage.misc.preparser as pre
    pre.load_attach_path = [ ... ]
 }}}

 Still, this is not very nice, and doesn't really fit into the "import
 sage.all" scheme. A third possibility is to "decide" that we might very
 well need such global variables in other places, stemming from various
 other foundational modules, and therefore introduce a new module
 sage.misc.globals to hold these. When Sage launches, it imports this
 module as "globals", and so, for any global variable - say
 load_attach_path -, the user would access it and modify it as
 globals.load_attach_path. I don't think that would be unnatural at all.
 However, it might be overkill if load_attach_path is the _only_ such
 conceivable global value, but for some reason (perhaps the sheer size of
 Sage), I can't really think it is.

 If none of these sound compelling, then I vote yes on the current form (I
 haven't tested the patch, that is). My only comment on the current patch,
 is that maybe there should be a TODO (or a fix ;-) ) in
 reset_load_attach_path(), which notes that probably the elements of
 SAGE_LOAD_ATTACH_PATH should be uniquified while preserving the order
 (thus, list(set(_load_attach_path)) is no good). After that the while
 could be reverted to an if again when removing the empty element.

 Otherwise good work. I'm looking forward to this patch in Sage, whatever
 the format :-)
 Cheers, Johan

-- 
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/378#comment:21>
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 post to this group, send email to [email protected].
To unsubscribe from this group, send email to 
[email protected].
For more options, visit this group at 
http://groups.google.com/group/sage-trac?hl=en.

Reply via email to