#33213 closed enhancement (fixed)
Replace SAGE_TMP by the system location in the sage library
Reported by:  Michael Orlitzky  Owned by:  

Priority:  minor  Milestone:  sage9.7 
Component:  misc  Keywords:  
Cc:  Samuel Lelièvre, Travis Scrimshaw, François Bissey  Merged in:  
Authors:  Michael Orlitzky  Reviewers:  Matthias Koeppe, Dima Pasechnik 
Report Upstream:  N/A  Work issues:  
Branch:  d94a1e6 (Commits, GitHub, GitLab)  Commit:  
Dependencies:  #33829, #33790  Stopgaps: 
Description (last modified by )
Re: https://groups.google.com/g/sagedevel/c/zhjl_j6j_Qc
These days, using a SAGE_TMP
that by default lives under $HOME
is overly complex and often inefficient. In this ticket we implement the first phase of its removal, to be replaced by python's tempfile
module. Specifically,
 We replace all direct uses of
SAGE_TMP
within the sage library and doctests.  We update
tmp_dir()
andtmp_filename()
to use thetempfile
defaults.  We remove
SAGE_TMP
.
Afterward, the custom functions tmp_dir()
and tmp_filename()
can be deprecated in favor of tempfile.TemporaryDirectory()
and tempfile.NamedTemporaryFile()
.
Moreover when #8784 is done, we'll be able to remove sagecleaner entirely.
Change History (97)
comment:1 Changed 8 months ago by
Branch:  → u/mjo/ticket/33213 

Commit:  → ffd9aa6a082cc6195efc1977a1d9f23b8d73a91e 
Description:  modified (diff) 
Summary:  Avoid SAGE_TMP for truly transient files and directories → Replace SAGE_TMP by the system location in the sage library 
comment:2 Changed 8 months ago by
Commit:  ffd9aa6a082cc6195efc1977a1d9f23b8d73a91e → f2b85e2dd38fd78ff300ec31da462c946332568a 

comment:3 Changed 8 months ago by
Commit:  f2b85e2dd38fd78ff300ec31da462c946332568a → 0f90d98f5782a25d10b5ae1797953673cb7dc468 

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
976d88b  Trac #33213: remove SAGE_TMP from tmp_dir() and tmp_filename().

8e09502  Trac #33213: don't clear SAGE_TMP upon exiting.

a22fa71  Trac #33213: remove SAGE_TMP from sage.misc.all.

810c855  Trac #33213: replace SAGE_TMP in cleaner/quit interfaces.

ce5a76d  Trac #33213: drop SAGE_TMP from sage.misc.misc.

955fdba  Trac #33213: remove the sagecleaner program.

a9df953  Trac #33213: remove "remote cleaner" option from expect interface.

9f504b5  Trac #33213: remove sage.interfaces.cleaner.

92e84da  Trac #33213: close an open file a bit sooner in gap interface.

0f90d98  Trac #33213: use @cached_function for the spawned processes file.

comment:4 Changed 8 months ago by
Dependencies:  → #8784 

Description:  modified (diff) 
Making #8784 a dependency because we can avoid many contortions on this branch if we don't have to keep sagecleaner aware of where the spawned_processes
file lives.
comment:5 followup: 6 Changed 8 months ago by
How did you solve the problems that sagecleaner solves? It's not like I wrote it for no reason. There are many situations in practice that people hit where temp files and subprocess get left around. Make sure that at least the following works:
 Start sage. Do something that creates a temp file and also something that spawns a subprocess.
 Segfault the sage that you started, e.g., by doing something stupid with Cython.
What happens? Are all temp files and spawned processes properly cleaned up?
It's an unfortunate reality that we make Cython relatively easy to use, hence it's easy from the command line or notebook to segfault a running Sage process.
Just curious. I never liked or wanted to write sagecleaner, but I couldn't think of any better approach to make end users happy.
comment:6 Changed 8 months ago by
Replying to was:
How did you solve the problems that sagecleaner solves? It's not like I wrote it for no reason. There are many situations in practice that people hit where temp files and subprocess get left around. Make sure that at least the following works:
 Start sage. Do something that creates a temp file and also something that spawns a subprocess.
 Segfault the sage that you started, e.g., by doing something stupid with Cython.
What happens? Are all temp files and spawned processes properly cleaned up?
 Now that all temporary files are created in
/tmp
or equivalent, they'll be cleaned up by the system eventually. In the event of a crash, I would say that this is preferable, because those temporary files can contain important debugging information.
 Sage doesn't segfault. More seriously, it depends on the process. In the simplest case, the subprocess will continue doing whatever it's doing until it completes, and will then terminate. Next would be subprocesses that communicate with the parent; eventually they'll be sent a SIGPIPE and commit suicide. If you go out of your way to launch something that is basically a daemon, though, it can indeed outlive the parent.
For (2) to be an issue, you have to launch a standalone process that never terminates, detach it from the parent, compile a C extension, load the extension into your current session, and then cause it to segfault. I don't think cleaning up after that situation should be the responsibility of any application; but especially not of a computer algebra system.
If you intend to do something like that frequently (or host a service for users who will...), there are better solutions these days  cgroups on linux being the best example. Keeping in mind that sagecleaner works by parsing a spawned_processes
file that must be updated every time a process is launched, solving the problem at the OS level in particular has the advantage that it will catch subsubprocesses. Sagecleaner generally cannot (unless the subprocess is also sage), because the subprocess doesn't know that it's supposed to add a line item to spawned_processes
.
comment:7 Changed 8 months ago by
I should also point out that it's not critical that we remove sagecleaner at this point. The main hurdle wrt the removal of SAGE_TMP
is that sagecleaner and the sage library (register_spawned_process()
) need to agree on a location for the spawned processes file. The current branch uses a temporary file in the library that lives for the duration of the sage session, and that's of no use to sagecleaner due to the random name. We could still store the spawned processes file with a fixed name in DOT_SAGE
, though, probably with the current sage PID tacked onto the path somewhere.
I saw this as a good time to remove it because its one crucial job  to clean up SAGE_TMP
under $HOME
 is now obsolete.
comment:8 Changed 8 months ago by
Commit:  0f90d98f5782a25d10b5ae1797953673cb7dc468 → de4a00ee85328833e7ce679a7c86be9aee3e489f 

Branch pushed to git repo; I updated commit sha1. New commits:
de4a00e  Trac #33213: ensure temporary gap workspace is created in a doctest.

comment:9 Changed 8 months ago by
Status:  new → needs_review 

I believe I've fixed all of the test failures so I'm setting this to needs_review
to see what the patchbots think.
comment:10 Changed 8 months ago by
comment:11 followup: 12 Changed 8 months ago by
Thanks @mjo for your explanation. Your responses don't necessarily apply in the context of users that got me to write the sagecleaner in the first place, and they will probably be pretty annoyed if they ever realize what happened. Sage is different than some more less complicated computer algebra software like Magma, because Sage much more frequently spawns subprocesses. Fortunately, over the years the number of subprocesses Sage spawns has been reduced due to writing and using C library interfaces instead of pexpect, etc.
Regarding /tmp, unless you specifically set something up to periodically clean up files, unless things have changed a lot, don't most Linux distros *not* automatically delete files from there, possibly until a reboot? You're making a huge change from "temp files get cleaned up by Sage itself quickly" to "maybe someday the system will clean them up". Maybe at this point that is the right change, but it's important to acknowledge that it is a significant change.
I agree that when users are using Linux, have sufficient privileges to use containers, and also actually know how to use them, then these same problems can be solved in a more robust way. However, this is a small percentage of users of Sage. Many users use other operating systems or don't have admin permissions.
CoCalc? is the main platform for running Sage that I care about these days, and every individual "project" is its own container, /tmp is extremely ephemeral, etc., so everything you say applies there. That said, users of CoCalc? can't use Docker or other container mechanisms themselves within projects, due to security constraints.
In any case, I'm not personally worried about these changes having any adverse impact for me. However, I'm leaving this comment here so people who are impacted can maybe know they may have to revert these patches for their own install of Sage. Probably the way things will go is that this gets merged, and a year later certain environments start finding that Sage is leaving around clutter in a way that Sage didn't for the last decade, but users don't know why. Hopefully they google and find this comment.
comment:12 Changed 8 months ago by
Replying to was:
Regarding /tmp, unless you specifically set something up to periodically clean up files, unless things have changed a lot, don't most Linux distros *not* automatically delete files from there, possibly until a reboot? You're making a huge change from "temp files get cleaned up by Sage itself quickly" to "maybe someday the system will clean them up". Maybe at this point that is the right change, but it's important to acknowledge that it is a significant change.... Probably the way things will go is that this gets merged, and a year later certain environments start finding that Sage is leaving around clutter in a way that Sage didn't for the last decade, but users don't know why. Hopefully they google and find this comment.
Systemd comes with a timer (like a cron job) to clean up /tmp
, but I'm not familiar with its outofthebox configuration on mainstream distros. I still use OpenRC myself, where a reboot is what triggers it.
On systems like mine, "leftover junk" is a fair criticism, but keep in mind the bigger picture: the python tempfile.TemporaryDirectory()
and tempfile.NamedTemporaryFile()
functions automatically remove the file/directory as soon as it's done being used. In this branch, I've changed a bunch of manual SAGE_TMP
code to use those two functions, so a lot of what would have been leftover junk is now simply not created in the first place. Any remaining junk is due to sage's own tmp_filename()
and tmp_dir()
, which expect you to clean up the file yourself, or assume that sagecleaner will do it eventually.
If this gets merged, we can deprecate tmp_filename()
and tmp_dir()
as obsolete in favor of the python functions. Over a hopefullyshort amount of time, that should eliminate the leftover junk at the source.
comment:13 followup: 14 Changed 8 months ago by
indeed, I am not at all worried about junk left at /tmp
, as, at least as far as documentation of Python is correct, the facility of NamedTemporaryFile cleans up after itself automatically (I guess it uses POSIX's tmpfile(3)
behind the scene).
I am a bit puzzled by the need to manually keep them open using delete=False
the call to NamedTemporaryFile
. Is it because one wants to keep them for the duration of a Sage session?
comment:14 followup: 15 Changed 8 months ago by
Replying to dimpase:
I am a bit puzzled by the need to manually keep them open using
delete=False
the call toNamedTemporaryFile
. Is it because one wants to keep them for the duration of a Sage session?
If you're referring to my new code (in many doctests), then this is simply a precaution for cygwin (see comment:1). I would love to simplify it if someone with a cygwin installation says that the "on Windows" warning doesn't apply there.
comment:15 Changed 8 months ago by
Replying to mjo:
If you're referring to my new code (in many doctests), then this is simply a precaution for cygwin (see comment:1). I would love to simplify it if someone with a cygwin installation says that the "on Windows" warning doesn't apply there.
(If it DOES apply on cygwin, we could improve this in the future by updating the functions that now take a filename to also accept an open file handle; that will avoid reopen()
ing it.)
comment:16 followup: 17 Changed 8 months ago by
Cygwin is POSIX emulator, so on Windows
doesn't really apply to it.
comment:17 Changed 8 months ago by
Replying to dimpase:
Cygwin is POSIX emulator, so
on Windows
doesn't really apply to it.
You have two wishes remaining =)
I'm "happy" to go back and change all of those doctests, but I'd like confirmation before I spend half a day doing it.
comment:18 followups: 19 32 Changed 8 months ago by
Cc:  Samuel Lelièvre added 

Perhaps slelievre can test this on Cygwin  if you specify in some detail what to look for.
comment:19 Changed 8 months ago by
Replying to dimpase:
Perhaps slelievre can test this on Cygwin  if you specify in some detail what to look for.
We just need to know if this works,
sage: import tempfile sage: p = plot(sin(x), x, (pi, pi)) sage: with tempfile.NamedTemporaryFile(suffix=".png") as f: ....: p.save(filename=f.name) ....:
or if the fact that save()
reopens the (already open) file causes problem.
comment:20 Changed 8 months ago by
Commit:  de4a00ee85328833e7ce679a7c86be9aee3e489f → 989a4cb43fc711eef66b75844b375ed1a7decf81 

Branch pushed to git repo; I updated commit sha1. New commits:
989a4cb  Trac #33213: clean up after tmp_filename() and tmp_dir() for now.

comment:21 Changed 8 months ago by
That last commit should avoid most leftover junk in the meantime, too. It's easy so why not.
comment:22 Changed 8 months ago by
comment:23 Changed 8 months ago by
Hi everybody  thanks for the comments. I just want to be clear that I am in no way suggesting blocking this ticket moving forward with a positive review. I just want to record my misgivings here, so they may be helpful for anybody that might hit problems as a result. I'm not a fan of this stupid sagecleaner hack that I wrote long ago, and I'm happy to see it get deleted.
comment:24 followups: 25 34 Changed 8 months ago by
The changes to using TemporaryDirectory
in doctests are easy to review and are definitely a great improvement that we should merge as soon as possible.
However, I would suggest to split this ticket, so that the wholesale removal of the cleaner is in its own ticket. In particular, user code may continue to use SAGE_TMP
, as this was the advertised interface since the beginning of time. So we need a transition period here. Perhaps a deprecation warning that the cleaner issues when it actually finds something to clean.
comment:25 followups: 26 68 Changed 8 months ago by
Replying to mkoeppe:
The changes to using
TemporaryDirectory
in doctests are easy to review and are definitely a great improvement that we should merge as soon as possible.However, I would suggest to split this ticket, so that the wholesale removal of the cleaner is in its own ticket. In particular, user code may continue to use
SAGE_TMP
, as this was the advertised interface since the beginning of time. So we need a transition period here. Perhaps a deprecation warning that the cleaner issues when it actually finds something to clean.
To my surprise, SAGE_TMP
was actually undocumented. It's not in the reference manual or the developer guide, and it had no docstring. But you're probably right that a few sage developers managed to use it in their own code.
If we want to raise a deprecation warning for it, I can do so in a way that doesn't affect the rest of this ticket by changing SAGE_TMP
to return a tempfile.TemporaryDirectory()
instead of the old DOT_SAGE
value. Then its use won't create anything that would require sagecleaner to remove.
comment:26 followup: 27 Changed 8 months ago by
I'm still 1 on removing the sagecleaner
here on the same ticket.
In particular I don't think the explanation why the processkilling part of it should suddenly no longer be necessary is very convincing. Let's please not introduce big changes like this without a proper transition plan.
comment:27 Changed 8 months ago by
comment:28 followup: 29 Changed 8 months ago by
Let's confirm this in practice please by doing the deprecation warnings whenever the cleaner kills or deletes something.
comment:29 followup: 31 Changed 8 months ago by
Replying to mkoeppe:
Let's confirm this in practice please by doing the deprecation warnings whenever the cleaner kills or deletes something.
It doesn't really delete anything except the files created specifically for the benefit of sagecleaner, and some legacy paths whose locations were changed in the fix_old_mistakes()
funtion.
I'm not strongly opposed to leaving sagecleaner alone, but a deprecation warning on killed processes is going to produce false positives on every process that would have terminated nicely but was killed prematurely by sagecleaner, i.e. most of them.
comment:30 Changed 8 months ago by
Perhaps it would be interesting to know whether after the changes (but keeping the cleaner in) the cleaner would still have anything to kill. (my understanding it starts after Sage terminates, so most likely it won't have to do anything)
comment:31 Changed 8 months ago by
comment:32 followup: 33 Changed 8 months ago by
Replying to dimpase:
Perhaps slelievre can test this on Cygwin
My attempts to build 9.5 beta and rc
releases fail on sagemath_doc_html
.
My attempts to build the 9.6 beta releases
fail earlier, on sagelib
I think.
comment:33 followup: 35 Changed 8 months ago by
Replying to slelievre:
My attempts to build the 9.6 beta releases fail earlier, on
sagelib
I think.
Fortunately this is only a python question... if you don't mind, does this work?
>>> import tempfile >>> with tempfile.NamedTemporaryFile(suffix=".txt") as f: ... with open(f.name, "w") as g: ... _ = g.write("Hello, world!") ... with open(f.name) as h: ... contents = h.read() ... >>> print(contents) Hello, world!
comment:34 Changed 8 months ago by
comment:35 followup: 36 Changed 7 months ago by
Replying to mjo:
does this work?
>>> import tempfile >>> with tempfile.NamedTemporaryFile(suffix=".txt") as f: ... with open(f.name, "w") as g: ... _ = g.write("Hello, world!") ... with open(f.name) as h: ... contents = h.read() ... >>> print(contents) Hello, world!
Yes, it works in Cygwin with Python 3.7.12, Python 3.8.12, Python 3.9.10.
comment:36 Changed 7 months ago by
Replying to slelievre:
Yes, it works in Cygwin with Python 3.7.12, Python 3.8.12, Python 3.9.10.
Thank you! That means I can go back and simplify a lot of stuff.
comment:37 Changed 7 months ago by
Commit:  989a4cb43fc711eef66b75844b375ed1a7decf81 → b9101fd945428f71ef3873df924496be0c211067 

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
56ebd57  Trac #33213: don't delete SAGE_TMP upon exiting.

c6b6fb8  Trac #33213: remove SAGE_TMP from sage.misc.all.

888aeb3  Trac #33213: replace SAGE_TMP in cleaner/quit interfaces.

f43965b  Trac #33213: drop SAGE_TMP from sage.misc.misc.

b9d1746  Trac #33213: remove sage.interfaces.cleaner.

e445eb8  Trac #33213: close an open file a bit sooner in gap interface.

049ce98  Trac #33213: use @cached_function for the spawned processes file.

e289f0b  Trac #33213: ensure temporary gap workspace is created in a doctest.

7dd9ff0  Trac #33213: clean up after tmp_filename() and tmp_dir() for now.

b9101fd  Trac #33213: use old SAGE_TMP path for spawned_processes file.

comment:38 Changed 7 months ago by
Dependencies:  #8784 → 32987 

Description:  modified (diff) 
Status:  needs_review → needs_work 
comment:39 Changed 7 months ago by
Dependencies:  32987 → #32987 

comment:40 Changed 7 months ago by
Commit:  b9101fd945428f71ef3873df924496be0c211067 → 86e654bb37ea223e62976a268c2b66107fe30e49 

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
518b8cc  Trac #33213: don't delete SAGE_TMP upon exiting.

754be97  Trac #33213: remove SAGE_TMP from sage.misc.all.

8c99287  Trac #33213: replace SAGE_TMP in cleaner/quit interfaces.

e2687dc  Trac #33213: drop SAGE_TMP from sage.misc.misc.

2ef1cb8  Trac #33213: remove sage.interfaces.cleaner.

98722ba  Trac #33213: close an open file a bit sooner in gap interface.

69e0f92  Trac #33213: use @cached_function for the spawned processes file.

6534cc1  Trac #33213: ensure temporary gap workspace is created in a doctest.

e4f0797  Trac #33213: clean up after tmp_filename() and tmp_dir() for now.

86e654b  Trac #33213: use old SAGE_TMP path for spawned_processes file.

comment:41 Changed 7 months ago by
Status:  needs_work → needs_review 

Ok, back to patchbots.
This branch is based on #32987 which deprecates sage_makedirs
(also now redundant), and does not remove sagecleaner. We can do that in another ticket.
comment:42 Changed 7 months ago by
Authors:  → Michael Orlitzky 

comment:43 followup: 48 Changed 7 months ago by
This one needs to be reverted:
diff git a/src/doc/en/reference/interfaces/index.rst b/src/doc/en/reference/interfaces/index.rst index d007f67..f09fc62 100644  a/src/doc/en/reference/interfaces/index.rst +++ b/src/doc/en/reference/interfaces/index.rst @@ 108,7 +108,6 @@ and testing to make sure nothing funny is going on). sage/interfaces/tachyon sage/interfaces/tides  sage/interfaces/cleaner sage/interfaces/quit sage/interfaces/read_data
comment:44 followup: 49 Changed 7 months ago by
Should tempfile
become a global import (sage.all
) to make the repeated imports in doctests unnecessary?
comment:45 followup: 50 Changed 7 months ago by
Also src/sage/interfaces/expect.py
still has cleanerrelated changes
comment:46 Changed 7 months ago by
Commit:  86e654bb37ea223e62976a268c2b66107fe30e49 → e6126787dec8007c9af93262d0b9c976d71b6b0e 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
72ed289  Trac #33213: move sage.interfaces.cleaner.cleaner() to quit interface.

0a75831  Trac #33213: close an open file a bit sooner in gap interface.

6b7322b  Trac #33213: use @cached_function for the spawned processes file.

c70d5f6  Trac #33213: ensure temporary gap workspace is created in a doctest.

ca764c6  Trac #33213: clean up after tmp_filename() and tmp_dir() for now.

e612678  Trac #33213: use old SAGE_TMP path for spawned_processes file.

comment:47 Changed 7 months ago by
Commit:  e6126787dec8007c9af93262d0b9c976d71b6b0e → ea321c961ffd41ca483c9b489b59d064cf203b1c 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
a0cdbcd  Trac #33213: move sage.interfaces.cleaner.cleaner() to quit interface.

e0961af  Trac #33213: close an open file a bit sooner in gap interface.

2123287  Trac #33213: use @cached_function for the spawned processes file.

50cf662  Trac #33213: ensure temporary gap workspace is created in a doctest.

465281a  Trac #33213: clean up after tmp_filename() and tmp_dir() for now.

ea321c9  Trac #33213: use old SAGE_TMP path for spawned_processes file.

comment:48 Changed 7 months ago by
Replying to mkoeppe:
This one needs to be reverted:
Thanks, done. I also removed the cleaner()
function from sage.interfaces.cleaner
, since it was replaced by register_spawned_process()
in sage.interfaces.quit
.
comment:49 followup: 51 Changed 7 months ago by
Replying to mkoeppe:
Should
tempfile
become a global import (sage.all
) to make the repeated imports in doctests unnecessary?
I don't have a strong opinion either way. The main downside is that it would clutter the global namespace for something that is pretty much only used for doctests. On the other hand, if we import it automatically only during doctesting, then the examples can't be copy & pasted.
comment:50 Changed 7 months ago by
Replying to mkoeppe:
Also
src/sage/interfaces/expect.py
still has cleanerrelated changes
Which one? The change from cleaner()
to register_spawned_process()
is intentional, and will make it easier to remove the cleaner in the future.
comment:51 Changed 7 months ago by
Replying to mjo:
if we import it automatically only during doctesting
definitely we shouldn't do that
comment:52 Changed 6 months ago by
Status:  needs_review → needs_work 

comment:53 Changed 6 months ago by
Commit:  ea321c961ffd41ca483c9b489b59d064cf203b1c → e8b8fa568a2cbbca6001f8890dd726c0c2c4882c 

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
8c1ea7d  Trac #33213: don't delete SAGE_TMP upon exiting.

c83b811  Trac #33213: remove SAGE_TMP from sage.misc.all.

c31d7d5  Trac #33213: replace SAGE_TMP in cleaner/quit interfaces.

d86a3e6  Trac #33213: drop SAGE_TMP from sage.misc.misc.

ac0dea7  Trac #33213: move sage.interfaces.cleaner.cleaner() to quit interface.

b268fd9  Trac #33213: close an open file a bit sooner in gap interface.

8ce0515  Trac #33213: use @cached_function for the spawned processes file.

3bfd7f9  Trac #33213: ensure temporary gap workspace is created in a doctest.

9f8a2f1  Trac #33213: clean up after tmp_filename() and tmp_dir() for now.

e8b8fa5  Trac #33213: use old SAGE_TMP path for spawned_processes file.

comment:54 Changed 6 months ago by
Status:  needs_work → needs_review 

comment:55 Changed 6 months ago by
sage t long randomseed=286814129660227055850621679081389420825 src/sage/misc/latex.py ********************************************************************** File "src/sage/misc/latex.py", line 1970, in sage.misc.latex.? Failed example: with tempfile.NamedTemporaryFile(suffix=".png") as f: png(ZZ[x], f.name) # random, optional  latex imagemagick Exception raised:
the # optional
is on the wrong line
comment:56 Changed 6 months ago by
Status:  needs_review → needs_work 

comment:57 Changed 6 months ago by
Commit:  e8b8fa568a2cbbca6001f8890dd726c0c2c4882c → a3b58ad4771287bfcc7f2952a56b67b3e075daaa 

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
63ac9b8  Trac #33213: don't delete SAGE_TMP upon exiting.

e191da6  Trac #33213: remove SAGE_TMP from sage.misc.all.

0498a50  Trac #33213: replace SAGE_TMP in cleaner/quit interfaces.

b90a7eb  Trac #33213: drop SAGE_TMP from sage.misc.misc.

279fe33  Trac #33213: move sage.interfaces.cleaner.cleaner() to quit interface.

c9b00b8  Trac #33213: close an open file a bit sooner in gap interface.

e4a82e7  Trac #33213: use @cached_function for the spawned processes file.

063b3fe  Trac #33213: ensure temporary gap workspace is created in a doctest.

386b6a5  Trac #33213: clean up after tmp_filename() and tmp_dir() for now.

a3b58ad  Trac #33213: use old SAGE_TMP path for spawned_processes file.

comment:59 followup: 62 Changed 6 months ago by
 a/src/sage/repl/attach.py +++ b/src/sage/repl/attach.py @@ 170,12 +170,14 @@ def load_attach_path(path=None, replace=False): ... + We put a new, empty direcoty on the attach path for testing
"directory"
comment:60 followup: 63 Changed 6 months ago by
Cc:  Travis Scrimshaw added 

Overall, I'm a bit unsure about changes like this one to the doctests. What is lost for the user is being able to copypaste the code and then inspect the generated file.
diff git a/src/sage/numerical/mip.pyx b/src/sage/numerical/mip.pyx index d966be1..4bf61d1 100644  a/src/sage/numerical/mip.pyx +++ b/src/sage/numerical/mip.pyx @@ 1336,7 +1336,9 @@ cdef class MixedIntegerLinearProgram(SageObject): sage: x = p.new_variable(nonnegative=True) sage: p.set_objective(x[1] + x[2]) sage: p.add_constraint(3*x[1] + 2*x[2], max=2,name="OneConstraint")  sage: p.write_mps(os.path.join(SAGE_TMP, "lp_problem.mps")) + sage: import tempfile + sage: with tempfile.NamedTemporaryFile(suffix=".mps") as f: + ....: p.write_mps(f.name) Writing problem data to ... 17 records were written
comment:61 Changed 6 months ago by
Commit:  a3b58ad4771287bfcc7f2952a56b67b3e075daaa → 0c59d5d1e5ed73ec2332a130054e287a4c346919 

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
eb1bd48  Trac #33213: don't delete SAGE_TMP upon exiting.

87d50be  Trac #33213: remove SAGE_TMP from sage.misc.all.

7434ae0  Trac #33213: replace SAGE_TMP in cleaner/quit interfaces.

738a684  Trac #33213: drop SAGE_TMP from sage.misc.misc.

7bf0a5d  Trac #33213: move sage.interfaces.cleaner.cleaner() to quit interface.

74dfbd5  Trac #33213: close an open file a bit sooner in gap interface.

f27612f  Trac #33213: use @cached_function for the spawned processes file.

d097e2f  Trac #33213: ensure temporary gap workspace is created in a doctest.

3b16d2e  Trac #33213: clean up after tmp_filename() and tmp_dir() for now.

0c59d5d  Trac #33213: use old SAGE_TMP path for spawned_processes file.

comment:63 Changed 6 months ago by
Replying to mkoeppe:
Overall, I'm a bit unsure about changes like this one to the doctests. What is lost for the user is being able to copypaste the code and then inspect the generated file.
In many cases, the point of the method is that it writes what would normally be shown to the user to a file instead. If you want to see this MILP, for example, then p.show()
can be used instead. I'm dodging your point, but looking back, many of these tests are in the same spirit.
In general, I would say it's bad practice to give users code that will write to a temporary file and teach them to expect it to remain there. It's better to have these copy/paste examples fail immediately, so that the user will pick a nontemporary path for his own experiments. Otherwise he could lose some work when sage exits or when the machine reboots.
comment:64 Changed 6 months ago by
Milestone:  sage9.6 → sage9.7 

comment:65 Changed 5 months ago by
Commit:  0c59d5d1e5ed73ec2332a130054e287a4c346919 → 20bc0cfeb5a007022d010e5fb300ffcb304ee770 

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
08430e0  Trac #33213: don't delete SAGE_TMP upon exiting.

058907e  Trac #33213: remove SAGE_TMP from sage.misc.all.

ad1dbd8  Trac #33213: replace SAGE_TMP in cleaner/quit interfaces.

f57fc07  Trac #33213: drop SAGE_TMP from sage.misc.misc.

aacc73c  Trac #33213: move sage.interfaces.cleaner.cleaner() to quit interface.

615f6e5  Trac #33213: close an open file a bit sooner in gap interface.

9f7d581  Trac #33213: use @cached_function for the spawned processes file.

9843b64  Trac #33213: ensure temporary gap workspace is created in a doctest.

47f9294  Trac #33213: clean up after tmp_filename() and tmp_dir() for now.

20bc0cf  Trac #33213: use old SAGE_TMP path for spawned_processes file.

comment:66 Changed 5 months ago by
Cc:  François Bissey added 

comment:67 Changed 5 months ago by
Dependencies:  #32987 → #33779 

I've split out #33779: Remove use of SAGE_TMP
in sage.interfaces.latte
comment:68 Changed 5 months ago by
Replying to mjo:
Replying to mkoeppe:
user code may continue to use
SAGE_TMP
, as this was the advertised interface since the beginning of time. So we need a transition period here. [...]To my surprise,
SAGE_TMP
was actually undocumented. It's not in the reference manual or the developer guide, and it had no docstring. But you're probably right that a few sage developers managed to use it in their own code.
Current branch still removes SAGE_TMP
. This needs to be backed out. Likewise the removal of ECL_TMP
, SPYX_TMP
 they are part of the public API.
comment:69 Changed 5 months ago by
Commit:  20bc0cfeb5a007022d010e5fb300ffcb304ee770 → 5a9edd957b014a3445aefd59313d3435bc5d4dda 

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
a3ecdce  Trac #33213: don't delete SAGE_TMP upon exiting.

f6cb517  Trac #33213: remove SAGE_TMP from sage.misc.all.

7e0e3c0  Trac #33213: replace SAGE_TMP in cleaner/quit interfaces.

218055b  Trac #33213: deprecate SAGE_TMP.

66268d6  Trac #33213: move sage.interfaces.cleaner.cleaner() to quit interface.

a373c84  Trac #33213: close an open file a bit sooner in gap interface.

ccbe766  Trac #33213: use @cached_function for the spawned processes file.

81a3bd8  Trac #33213: ensure temporary gap workspace is created in a doctest.

06202e3  Trac #33213: clean up after tmp_filename() and tmp_dir() for now.

5a9edd9  Trac #33213: use old SAGE_TMP path for spawned_processes file.

comment:70 Changed 5 months ago by
I replaced the removal of SAGE_TMP
by a deprecation, and likewise with the rename SPYX_TMP
> spyx_tmp()
.
I don't fully agree with doing the same for ECL_TMP
since sage doesn't use it any more: anyone using it will have their code implicitly broken rather than explicitly. Nevertheless, I doubt anyone is using it, so I've put it back with a deprecation and relocated the path somewhere under the system's temporary location.
comment:71 Changed 5 months ago by
Could you move the new functions spyx_tmp
to sage.misc.temporary_file
, like I tried in #32986? This would help with modularization
comment:72 Changed 5 months ago by
I've opened #33790 for the ECL/Maxima stuff  I'd like to run this by the ECL/Maxima people
comment:73 Changed 5 months ago by
Commit:  5a9edd957b014a3445aefd59313d3435bc5d4dda → 1a4968523eb9c2ecd66e7148e78e66db909c86ae 

Branch pushed to git repo; I updated commit sha1. New commits:
1a49685  Trac #33213: move spyx_tmp() to sage.misc.temporary_file.

comment:75 Changed 5 months ago by
I forgot to say I'll probably want to keep sage.misc.temporary_file
without a dependence on Cython modules such as cachefunc
comment:76 Changed 5 months ago by
Dependencies:  #33779 → #33779 #33793 

comment:77 Changed 5 months ago by
Commit:  1a4968523eb9c2ecd66e7148e78e66db909c86ae → a980ff2bb7b3ab895f25f60e11024ea225899cfc 

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
8c3da84  Trac #33213: don't delete SAGE_TMP upon exiting.

8fa1aa6  Trac #33213: remove SAGE_TMP from sage.misc.all.

e12408a  Trac #33213: replace SAGE_TMP in cleaner/quit interfaces.

a899a3f  Trac #33213: deprecate SAGE_TMP.

c810bf0  Trac #33213: move sage.interfaces.cleaner.cleaner() to quit interface.

f689fe3  Trac #33213: close an open file a bit sooner in gap interface.

0a39999  Trac #33213: use @cached_function for the spawned processes file.

54a49c4  Trac #33213: ensure temporary gap workspace is created in a doctest.

93d9b4c  Trac #33213: clean up after tmp_filename() and tmp_dir() for now.

a980ff2  Trac #33213: use old SAGE_TMP path for spawned_processes file.

comment:79 Changed 5 months ago by
Dependencies:  #33779 #33793 → #33779 #33793 #33797 #33799 

comment:80 Changed 5 months ago by
comment:81 Changed 4 months ago by
Dependencies:  #33779 #33793 #33797 #33799 → #33829 

comment:82 Changed 4 months ago by
Branch:  u/mjo/ticket/33213 → u/dimpase/ticket/33213 

Commit:  a980ff2bb7b3ab895f25f60e11024ea225899cfc → 722f56c6c9c961c9c4dcb3e956c61c2b9b16def5 
Status:  needs_review → needs_work 
need to get rid of SAGE_TMP in msolve.py
src/sage/rings/polynomial/msolve.py:from sage.misc.all import SAGE_TMP src/sage/rings/polynomial/msolve.py: msolve_in = tempfile.NamedTemporaryFile(dir=SAGE_TMP, mode='w', ...
Last 10 new commits:
9ee2c0d  Trac #33213: remove mentions of SAGE_TMP from the lazy_string module.

bab61df  Trac #33213: don't delete SAGE_TMP upon exiting.

9d56a0b  Trac #33213: remove SAGE_TMP from sage.misc.all.

95abd2b  Trac #33213: replace SAGE_TMP in cleaner/quit interfaces.

e875f1d  Trac #33213: deprecate SAGE_TMP.

7dd2ee0  Trac #33213: move sage.interfaces.cleaner.cleaner() to quit interface.

e386cee  Trac #33213: close an open file a bit sooner in gap interface.

595c082  Trac #33213: use @cached_function for the spawned processes file.

46ae7b2  Trac #33213: ensure temporary gap workspace is created in a doctest.

722f56c  Trac #33213: use old SAGE_TMP path for spawned_processes file.

comment:84 Changed 4 months ago by
Commit:  722f56c6c9c961c9c4dcb3e956c61c2b9b16def5 → beacc6853a162550cfd5cfd3ba9b004a181d5a6c 

Branch pushed to git repo; I updated commit sha1. New commits:
beacc68  do not use SAGE_TMP

comment:85 Changed 4 months ago by
Commit:  beacc6853a162550cfd5cfd3ba9b004a181d5a6c → 9afa20b82c54b44740469f37d0b8668280933056 

Branch pushed to git repo; I updated commit sha1. New commits:
9afa20b  add forgotten while rebasing deprecation in misc/dist.py

comment:86 Changed 4 months ago by
Reviewers:  → Matthias Koeppe, Dima Pasechnik 

Status:  needs_work → positive_review 
Matthias, is it OK with you?
comment:88 Changed 4 months ago by
comment:89 Changed 4 months ago by
Status:  positive_review → needs_review 

I haven't reviewed the changes on the ticket. The parts that I was able to review have been split out to separate tickets.
comment:90 Changed 4 months ago by
Dependencies:  #33829 → #33829, #33790 

Status:  needs_review → needs_work 
Work issues:  → Rebase on top of the dependencies 
comment:91 Changed 4 months ago by
Commit:  9afa20b82c54b44740469f37d0b8668280933056 → d94a1e62c1ce92ab65cd840b3f002101b00a36e5 

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
c8c9d40  Trac #33213: deprecate SAGE_TMP.

ec1a3cf  Trac #33213: move sage.interfaces.cleaner.cleaner() to quit interface.

78db2fd  Trac #33213: close an open file a bit sooner in gap interface.

56e2964  Trac #33213: use @cached_function for the spawned processes file.

31986c1  Trac #33213: ensure temporary gap workspace is created in a doctest.

c7ff254  Trac #33213: use old SAGE_TMP path for spawned_processes file.

78f029b  do not use SAGE_TMP

7ae0e41  add forgotten while rebasing deprecation in misc/dist.py

5d314d9  pkgs/sagesetup, sagemath{objects,categories,standard}: Add tox environment sagepython

d94a1e6  src/doc/en/developer/packaging_sage_library.rst: Use sagepython instead of hardcoded py39

comment:92 Changed 4 months ago by
Status:  needs_work → needs_review 

comment:93 Changed 4 months ago by
Work issues:  Rebase on top of the dependencies 

comment:94 Changed 4 months ago by
Status:  needs_review → positive_review 

comment:95 Changed 3 months ago by
Branch:  u/dimpase/ticket/33213 → d94a1e62c1ce92ab65cd840b3f002101b00a36e5 

Resolution:  → fixed 
Status:  positive_review → closed 
comment:96 Changed 3 months ago by
Commit:  d94a1e62c1ce92ab65cd840b3f002101b00a36e5 

The branch includes a rebased version of #32716
comment:97 Changed 6 days ago by
Followup discussion in https://groups.google.com/g/sagedevel/c/jpwUb8OCVVc
I've posted a workinprogress branch that raises a question. The python docs,
state that, in regards to a named temporary file,
So in many cases, where I would have liked to have done
I have instead done something like
or
to avoid the
save_image
call reopening the same name. However, the docs are unclear about what "on Windows" means. So the two examples above can be simplified if this is not a problem on cygwin.Last 10 new commits:
Trac #33213: replace SAGE_TMP in repl doctests.
Trac #33213: replace SAGE_TMP in magma interface doctests.
Trac #33213: replace SAGE_TMP in dist script doctests.
Trac #33213: remove mentions of SAGE_TMP from the lazy_string module.
Trac #33213: replace SAGE_TMP in session doctests.
Trac #33213: remove SAGE_TMP from tmp_dir() and tmp_filename().
Trac #33213: don't clear SAGE_TMP upon exiting.
Trac #33213: remove SAGE_TMP from sage.misc.all.
Trac #33213: replace SAGE_TMP in cleaner/quit interfaces.
Trac #33213: drop SAGE_TMP from sage.misc.misc.