sagemath/sage

Deprecate sage.misc.misc.sage_makedirs

mkoeppe opened this issue · 17 comments

As noted in #29093, in py3, os.makedirs learned an exist_ok option that makes sage_makedirs entirely redundant.

https://docs.python.org/3/library/os.html#os.makedirs

Since then we have dropped py2 support, so we can deprecate sage_makedirs and replace all uses in the library.

CC: @mezzarobba @orlitzky

Component: refactoring

Author: Matthias Koeppe

Branch/Commit: d3b3252

Reviewer: Kwankyu Lee, Michael Orlitzky

Issue created by migration from https://trac.sagemath.org/ticket/32987

Description changed:

--- 
+++ 
@@ -1,4 +1,6 @@
 As noted in #29093, in py3, `os.makedirs` learned an `exist_ok` option that makes `sage_makedirs` entirely redundant.
+
+https://docs.python.org/3/library/os.html#os.makedirs
 
 Since then we have dropped py2 support, so we can deprecate `sage_makedirs` and replace all uses in the library.
 

Dependencies: #32986

Commit: 37218da

New commits:

81d0fffsage.misc.temporary_file: Move SAGE_TMP implementation here
7e78f59src/sage/misc/temporary_file.py: Remove use of functools.cache
a9de636Merge #32986
52b810fgit grep -l sage_makedirs | xargs sed -E -i.bak 's/from sage.misc.misc import sage_makedirs/import os/;s/sage_makedirs[(](.*)[)]$/os.makedirs(\1, exist_ok=True)/'
5b94d90Remove duplicate imports
976ff6fRemove remaining import of sage_makedirs
37218dasage.misc.misc.sage_makedirs: Deprecate

Author: Matthias Koeppe

comment:5

It works well.

I see two doctest failures with src/sage/tests/cmdline.py, but these seem related with pytest issues, not with the current branch.

Reviewer: Kwankyu Lee

comment:6

Thank you!

Changed commit from 37218da to d3b3252

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. This was a forced push. New commits:

766d835git grep -l sage_makedirs | xargs sed -E -i.bak 's/from sage.misc.misc import sage_makedirs/import os/;s/sage_makedirs[(](.*)[)]$/os.makedirs(\1, exist_ok=True)/'
4494364Remove duplicate imports
41b015cRemove remaining import of sage_makedirs
e891787sage.misc.misc.sage_makedirs: Deprecate
d3b3252sage.misc.misc (SAGE_TMP, ECL_TMP, SAGE_TMP_INTERFACE): Also replace use of sage_makedirs here

Changed dependencies from #32986 to none

comment:9

Rebased away from #32986 (which is stuck)

Changed reviewer from Kwankyu Lee to Kwankyu Lee, Michael Orlitzky

comment:10

This was already positively reviewed, but LGTM too after rebasing.

comment:11

Thanks!