Opened 8 years ago
Last modified 8 years ago
#11577 closed defect
save(x,filename) fails for pure Python objects for x if filename contains a dot — at Version 9
Reported by: | logix | Owned by: | was |
---|---|---|---|
Priority: | minor | Milestone: | sage-4.7.2 |
Component: | pickling | Keywords: | .sobj |
Cc: | Merged in: | ||
Authors: | Leif Leonhardy | Reviewers: | John Palmieri |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
(The summary actually is not completely accurate - there might be some Python object this works for that I'm not aware of.)
If the filename passed to save()
contains a dot, save()
assumes that the user doesn't just want to dump the (pickled) object, but instead wants to call the object's save()
method. I guess this makes sense in situations like save(g, 'mygraph.png')
, but the code should fall back to dumping the pickled version (e.g. via try: ... except AttributeError: ...
- suggested via IRC by leif) if the object has no save()
method.
leif also suggested checking if the file name extension is known - however I guess that we then should verify this with the object itself (e.g. it wouldn't make sense to save a graphics object to a .wav
file) and not statically compare with a list of known extensions.
sage: save((1,1), 'foo2') sage: save(Matrix(3,3), 'foo.bar3') sage: save((1,1), 'foo.bar4') --------------------------------------------------------------------------- AttributeError Traceback (most recent call last) /tmp/sagedebug/<ipython console> in <module>() /usr/local/sage/local/lib/python2.6/site-packages/sage/structure/sage_object.so in sage.structure.sage_object.save (sage/structure/sage_object.c:8156)() AttributeError: 'tuple' object has no attribute 'save'
Here's the culprit:
# Add '.sobj' if the filename currently has no extension if os.path.splitext(filename)[1] == '': filename += '.sobj' if filename.endswith('.sobj'): try: obj.save(filename=filename, compress=compress, **kwds) except (AttributeError, RuntimeError, TypeError): s = cPickle.dumps(obj, protocol=2) if compress: s = comp.compress(s) open(process(filename), 'wb').write(s) else: # Saving an object to an image file. # XXX This of course fails for plain Python objects: obj.save(filename, **kwds)
Apply trac_11577-jhp.patch.
Change History (12)
comment:1 Changed 8 years ago by
- Description modified (diff)
- Summary changed from save(x,filename) fails for some types of objects for x if filename contains a dot to save(x,filename) fails for pure Python objects for x if filename contains a dot
comment:2 Changed 8 years ago by
- Component changed from sage-mode to pickling
- Description modified (diff)
- Owner changed from ncalexan to was
comment:3 Changed 8 years ago by
comment:4 Changed 8 years ago by
- Status changed from new to needs_review
Attached patch does exactly what I suggested last.
comment:5 Changed 8 years ago by
- Milestone changed from sage-4.7.2 to sage-4.7.1
comment:6 follow-up: ↓ 7 Changed 8 years ago by
- Status changed from needs_review to needs_work
You shouldn't write to a file like "foo.bar" in a doctest. Instead, make sure you write to a temporary directory, for example using SAGE_TMP.
comment:7 in reply to: ↑ 6 Changed 8 years ago by
Replying to jhpalmieri:
You shouldn't write to a file like "foo.bar" in a doctest. Instead, make sure you write to a temporary directory, for example using SAGE_TMP.
Ooops, I thought doctests would be executed there, i.e. sage{-doctest,doctest.py}
would cd
to that before running Python.
Changed 8 years ago by
Sage library patch. Adds ".sobj" to filename if the object has no save() method. (Corrected and improved version.) Based on Sage 4.7.1.rc0.
comment:8 Changed 8 years ago by
- Status changed from needs_work to needs_review
Finally...
The updated patch now adds another doctest and clarifies the description in the docstring. (I intentionally haven't changed it to enumerate all parameters as we normally do, as I think it is more readable as it is now, and compress=True
should be self-explanatory. For some reason I don't get the correct default parameters shown when typing save?
at the Sage prompt though.)
I originally wanted to add
import os.path from sage.misc.misc import SAGE_TMP filename = os.path.join(SAGE_TMP, "foo.bar") ...
but then saw the other examples, so I guess a lot of doctests rely on
- having
SAGE_TMP
already imported (bysage.all
), and also SAGE_TMP
having a trailing slash (oros.path.sep
),
the latter being perhaps a bit more dangerous than the former.
So I also just used SAGE_TMP + "..."
.
comment:9 Changed 8 years ago by
- Description modified (diff)
- Milestone changed from sage-4.7.1 to sage-4.7.2
- Reviewers set to John Palmieri
- Status changed from needs_review to positive_review
At the sage: prompt, I get
Definition: save(obj, filename, compress=None, **kwds=True)
Odd, the values are shifted right by one (filename should be None, compress should be True). I wonder what's causing that. Other functions in the same file seem to be okay. I have some old copies of Sage around, and this behavior dates back to at least version 4.3.
As far as SAGE_TMP
is concerned, you're right, it's sloppy. Probably the right solution is to have a function sage_tmpfile
or something like that, which takes one argument, FILE, and returns a valid path using os.path.join(SAGE_TMP, FILE)
. In fact, if you look at how SAGE_TMP
is defined in the first place (in sage.misc.misc), it's defined using explicit slashes rather than with os.path.join
. This all belongs on another ticket; if I have time, I'll work on that eventually.
I'm happy with the patch, basically as is. I'm attaching a new version fixing one or two words in the docstring. I'm marking it as "positive review"; if you don't like my changes, switch it back.
What about simply adding
.sobj
above if there's no extension or the object has nosave()
method?