Opened 3 months ago

Closed 8 weeks ago

#33829 closed enhancement (fixed)

sage.graphs: Do not use SAGE_TMP in doctests

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-9.7
Component: refactoring Keywords:
Cc: mjo, dcoudert, dimpase, chapoton Merged in:
Authors: Michael Orlitzky Reviewers: David Coudert, Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: 976f0d6 (Commits, GitHub, GitLab) Commit: 976f0d686ee5375ed3894009a13b2cd54fcd89f2
Dependencies: Stopgaps:

Status badges

Description

(split out from #33213)

Change History (20)

comment:1 Changed 3 months ago by mkoeppe

  • Branch set to u/mkoeppe/sage_graphs__do_not_use_sage_tmp_in_doctests

comment:2 Changed 3 months ago by mkoeppe

  • Authors set to Michael Orlitzky
  • Cc dcoudert dimpase added
  • Commit set to 354ed405936cff0b917d136942ae72561b72d39e
  • Status changed from new to needs_review

New commits:

680be5cTrac #33213: replace SAGE_TMP in the isgci database download routine.
354ed40Trac #33213: replace SAGE_TMP in GLPK graph backend doctests.

comment:3 Changed 3 months ago by git

  • Commit changed from 354ed405936cff0b917d136942ae72561b72d39e to aaae76894166c78a2247d7e23d1bcead2d6d5979

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

aaae768Trac #33213: replace SAGE_TMP in graph doctests.

comment:4 Changed 3 months ago by dcoudert

  • Reviewers set to David Coudert
  • Status changed from needs_review to positive_review

LGTM. All tests pass.

comment:5 Changed 3 months ago by vbraun

  • Status changed from positive_review to needs_work

Merge failure on top of:

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

b4009625f3 Trac #33824: make gens and orbits of PermutationGroup? immutable

af23627f18 Trac #33809: some pathlib in combinat and groups

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

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

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

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

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

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

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

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

b376a8d71c Trac #33771: SSLContext needs an argument

df168c8112 Trac #33763: Refactor src/sage/docs

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

f02236f5b6 Trac #33744: Compute bases/circuits in MatroidUnion?

8943dc07fc Trac #33743: Faster random tree generator

773ec37bec Trac #33740: Add conda dev environment

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

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

6f4efb0bf3 Updated SageMath version to 9.7.beta0

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

comment:6 Changed 3 months ago by mkoeppe

  • Dependencies set to #33771

comment:7 Changed 3 months ago by git

  • Commit changed from aaae76894166c78a2247d7e23d1bcead2d6d5979 to 152a457ef1d173624216ab669adfca8bb3215ab8

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

93e7f60no longer use SSLContext
152a457Merge #33771

comment:8 Changed 3 months ago by mkoeppe

Merged #33771 to resolve merge conflict

comment:9 Changed 3 months ago by mkoeppe

  • Cc chapoton added
  • Status changed from needs_work to needs_review

comment:10 Changed 3 months ago by dcoudert

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.

comment:11 Changed 2 months ago by dimpase

  • Reviewers changed from David Coudert to David Coudert, Dima Pasechnik
  • Status changed from needs_review to positive_review

lgtm

comment:12 Changed 2 months ago by mjo

  • Status changed from positive_review to needs_work

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

comment:13 Changed 2 months ago by dimpase

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.

comment:14 Changed 2 months ago by dimpase

  • Branch changed from u/mkoeppe/sage_graphs__do_not_use_sage_tmp_in_doctests to u/dimpase/sage_graphs__do_not_use_sage_tmp_in_doctests
  • Commit changed from 152a457ef1d173624216ab669adfca8bb3215ab8 to 1721333b2b8a1e9d930709cf9fbd1b2ede85fc4a
  • Status changed from needs_work to needs_review

New commits:

2dd0ae8Trac #33213: replace SAGE_TMP in the isgci database download routine.
4c49df2Trac #33213: replace SAGE_TMP in GLPK graph backend doctests.
9260d20Trac #33213: replace SAGE_TMP in graph doctests.
1721333remove crud after except, make it pass

comment:15 Changed 2 months ago by git

  • Commit changed from 1721333b2b8a1e9d930709cf9fbd1b2ede85fc4a to 976f0d686ee5375ed3894009a13b2cd54fcd89f2

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

818410fTrac #33213: replace SAGE_TMP in the isgci database download routine.
71c9325Trac #33213: replace SAGE_TMP in GLPK graph backend doctests.
08213faTrac #33213: replace SAGE_TMP in graph doctests.
976f0d6remove crud after except, make it pass

comment:16 Changed 2 months ago by dimpase

rebased over latest 9.7.beta2

comment:17 Changed 2 months ago by dimpase

  • Status changed from needs_review to positive_review

it works

comment:18 Changed 2 months ago by dimpase

  • Dependencies changed from #33771 to #33829

comment:19 Changed 2 months ago by dimpase

  • Dependencies #33829 deleted

comment:20 Changed 8 weeks ago by vbraun

  • Branch changed from u/dimpase/sage_graphs__do_not_use_sage_tmp_in_doctests to 976f0d686ee5375ed3894009a13b2cd54fcd89f2
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.