sage.graphs: Do not use SAGE_TMP in doctests

Question

sage.graphs: Do not use SAGE_TMP in doctests

mkoeppe opened this issue 4 years ago · 28 comments

mkoeppe commented 4 years ago

(split out from #33213)

CC: @orlitzky @dcoudert @dimpase @fchapoton

Component: refactoring

Author: Michael Orlitzky

Branch/Commit: 976f0d6

Reviewer: David Coudert, Dima Pasechnik

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

Answer 1 · 2022-05-09T23:27:37.000Z

Branch: u/mkoeppe/sage_graphs__do_not_use_sage_tmp_in_doctests

Answer 2 · 2022-05-09T23:28:10.000Z

Commit: 354ed40

Answer 3 · 2022-05-09T23:28:10.000Z

New commits:

`680be5c`	`Trac #33213: replace SAGE_TMP in the isgci database download routine.`
`354ed40`	`Trac #33213: replace SAGE_TMP in GLPK graph backend doctests.`

Answer 4 · 2022-05-09T23:28:10.000Z

Author: Michael Orlitzky

Answer 5 · 2022-05-09T23:29:00.000Z

Changed commit from 354ed40 to aaae768

Answer 6 · 2022-05-09T23:29:00.000Z

Branch pushed to git repo; I updated commit sha1. New commits:

`aaae768`	`Trac #33213: replace SAGE_TMP in graph doctests.`

Answer 7 · 2022-05-10T09:18:53.000Z

Reviewer: David Coudert

Answer 8 · 2022-05-10T09:18:53.000Z

comment:4

LGTM. All tests pass.

Answer 9 · 2022-05-22T10:26:21.000Z

comment:5

Merge failure on top of:

e2ebf44 Trac #33825: Use pytest-xdist to run pytest in parallel

b400962 Trac #33824: make gens and orbits of PermutationGroup immutable

af23627 Trac #33809: some pathlib in combinat and groups

955b5d9 Trac #33803: Fixes for the distributions sagemath-objects, sagemath-categories

3e6b41f Trac #33799: Replace SAGE_TMP in doctests of sage.misc.{persist,ostools}, sage.doctest, sage.repl

a3fd718 Trac #33797: sage.misc.temporary_file: Remove use of SAGE_TMP

2ca0530 Trac #33794: modules/fp_graded/morphism.py test failure

7037fba Trac #33793: sage.misc.cython: Replace use of SPYX_TMP by a new cached function in sage.misc.temporary_file

d115270 Trac #33787: Installation manual: Update section "system-wide install"

0ae5565 Trac #33782: ci-cygwin: Update python version

833f53d Trac #33779: Remove use of SAGE_TMP in sage.interfaces.latte

b376a8d Trac #33771: SSLContext needs an argument

df168c8 Trac #33763: Refactor src/sage/docs

9597eaf Trac #33748: make accessing coefficients of finite-field elements easier

f02236f Trac #33744: Compute bases/circuits in MatroidUnion

8943dc0 Trac #33743: Faster random tree generator

773ec37 Trac #33740: Add conda dev environment

5e65c16 Trac #33734: variety() for polynomial systems over ℚ using msolve

8e7dcca Trac #33733: allow to use flint for Stirling numbers

6f4efb0 Updated SageMath version to 9.7.beta0

merge was not clean: conflicts in src/sage/graphs/isgci.py

Answer 10 · 2022-05-25T20:49:45.000Z

Dependencies: #33771

Answer 11 · 2022-05-25T20:50:59.000Z

Changed commit from aaae768 to 152a457

Answer 12 · 2022-05-25T20:50:59.000Z

Branch pushed to git repo; I updated commit sha1. New commits:

`93e7f60`	`no longer use SSLContext`
`152a457`	`Merge #33771`

Answer 13 · 2022-05-25T20:51:12.000Z

comment:8

Merged #33771 to resolve merge conflict

Answer 14 · 2022-05-26T08:59:19.000Z

comment:10

in isgci.py in the following block you use variable d but this variable is unknown (reported by pyflakes)

+            # Save a systemwide updated copy whenever possible
+            try:
+                z.extract(_XML_FILE, GRAPHS_DATA_DIR)
+                z.extract(_SMALLGRAPHS_FILE, GRAPHS_DATA_DIR)
+            except IOError:
+                z.extract(_XML_FILE, d)
+                z.extract(_SMALLGRAPHS_FILE, GRAPHS_DATA_DIR)

pyflakes also complains about os that is imported but not used. This is a minor issue.

Answer 15 · 2022-06-10T09:20:33.000Z

comment:11

lgtm

Answer 16 · 2022-06-10T09:20:33.000Z

Changed reviewer from David Coudert to David Coudert, Dima Pasechnik

Answer 17 · 2022-06-10T11:40:39.000Z

comment:12

something should still be done with the isgci.py routine from comment:10

it looks like the fallback SAGE_TMP location never worked because _parse_db doesn't try the fallback location, so maybe we should just delete the whole except IOError: block

Answer 18 · 2022-06-10T16:56:26.000Z

comment:13

I'm trying to replace what's inside the except by pass. We can of course go even further and get rid of try all together.

Answer 19 · 2022-06-10T22:29:05.000Z

Changed branch from u/mkoeppe/sage_graphs__do_not_use_sage_tmp_in_doctests to u/dimpase/sage_graphs__do_not_use_sage_tmp_in_doctests

Answer 20 · 2022-06-10T22:29:05.000Z

Changed commit from 152a457 to 1721333

Answer 21 · 2022-06-10T22:29:05.000Z

New commits:

`2dd0ae8`	`Trac #33213: replace SAGE_TMP in the isgci database download routine.`
`4c49df2`	`Trac #33213: replace SAGE_TMP in GLPK graph backend doctests.`
`9260d20`	`Trac #33213: replace SAGE_TMP in graph doctests.`
`1721333`	`remove crud after except, make it pass`

Answer 22 · 2022-06-13T10:01:56.000Z

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

`818410f`	`Trac #33213: replace SAGE_TMP in the isgci database download routine.`
`71c9325`	`Trac #33213: replace SAGE_TMP in GLPK graph backend doctests.`
`08213fa`	`Trac #33213: replace SAGE_TMP in graph doctests.`
`976f0d6`	`remove crud after except, make it pass`

Answer 23 · 2022-06-13T10:01:56.000Z

Changed commit from 1721333 to 976f0d6

Answer 24 · 2022-06-13T10:06:57.000Z

comment:16

rebased over latest 9.7.beta2

Answer 25 · 2022-06-13T10:58:17.000Z

comment:17

it works

Answer 26 · 2022-06-13T11:00:36.000Z

Changed dependencies from #33771 to #33829

Answer 27 · 2022-06-13T17:38:18.000Z

Changed dependencies from #33829 to none

Answer 28 · 2022-06-17T22:52:58.000Z

Changed branch from u/dimpase/sage_graphs__do_not_use_sage_tmp_in_doctests to 976f0d6