Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sage.graphs: Do not use SAGE_TMP in doctests #33829

Closed
mkoeppe opened this issue May 9, 2022 · 28 comments
Closed

sage.graphs: Do not use SAGE_TMP in doctests #33829

mkoeppe opened this issue May 9, 2022 · 28 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented May 9, 2022

(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

@mkoeppe mkoeppe added this to the sage-9.7 milestone May 9, 2022
@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 9, 2022

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 9, 2022

New commits:

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

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 9, 2022

Commit: 354ed40

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 9, 2022

Author: Michael Orlitzky

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 9, 2022

Changed commit from 354ed40 to aaae768

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 9, 2022

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

aaae768Trac #33213: replace SAGE_TMP in graph doctests.

@dcoudert
Copy link
Contributor

Reviewer: David Coudert

@dcoudert
Copy link
Contributor

comment:4

LGTM. All tests pass.

@vbraun
Copy link
Member

vbraun commented May 22, 2022

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

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 25, 2022

Dependencies: #33771

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 25, 2022

Changed commit from aaae768 to 152a457

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 25, 2022

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

93e7f60no longer use SSLContext
152a457Merge #33771

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 25, 2022

comment:8

Merged #33771 to resolve merge conflict

@dcoudert
Copy link
Contributor

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.

@dimpase
Copy link
Member

dimpase commented Jun 10, 2022

comment:11

lgtm

@dimpase
Copy link
Member

dimpase commented Jun 10, 2022

Changed reviewer from David Coudert to David Coudert, Dima Pasechnik

@orlitzky
Copy link
Contributor

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

@dimpase
Copy link
Member

dimpase commented Jun 10, 2022

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.

@dimpase
Copy link
Member

dimpase commented Jun 10, 2022

@dimpase
Copy link
Member

dimpase commented Jun 10, 2022

Changed commit from 152a457 to 1721333

@dimpase
Copy link
Member

dimpase commented Jun 10, 2022

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 13, 2022

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 13, 2022

Changed commit from 1721333 to 976f0d6

@dimpase
Copy link
Member

dimpase commented Jun 13, 2022

comment:16

rebased over latest 9.7.beta2

@dimpase
Copy link
Member

dimpase commented Jun 13, 2022

comment:17

it works

@dimpase
Copy link
Member

dimpase commented Jun 13, 2022

Changed dependencies from #33771 to #33829

@dimpase
Copy link
Member

dimpase commented Jun 13, 2022

Changed dependencies from #33829 to none

@vbraun
Copy link
Member

vbraun commented Jun 17, 2022

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants