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

Replace sage.doctest.external.has_* functions by Features #32649

Closed
mkoeppe opened this issue Oct 7, 2021 · 71 comments
Closed

Replace sage.doctest.external.has_* functions by Features #32649

mkoeppe opened this issue Oct 7, 2021 · 71 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 7, 2021

Some of them already go through features - for those we just rename the feature so that its name attribute is equal to the intended "optional" tag.

For the remaining ones, we add new Feature subclasses in new files src/sage/features/internet.py, mip_backends.py.

Follow-ups:

Depends on #30887
Depends on #32614

CC: @kwankyu @jhpalmieri @seblabbe @dimpase @jdemeyer @saraedum @tscrim @dcoudert @sagetrac-tmonteil @videlec

Component: refactoring

Author: Matthias Koeppe, Kwankyu Lee

Branch: 1d02bd0

Reviewer: Kwankyu Lee, Matthias Koeppe, Sébastien Labbé

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

@mkoeppe mkoeppe added this to the sage-9.5 milestone Oct 7, 2021
@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 7, 2021

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 7, 2021

New commits:

1c6335asage.doctest.external.has_internet: Refactor through new Feature

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 7, 2021

Commit: 1c6335a

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 7, 2021

Author: Matthias Koeppe, ...

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 7, 2021

Dependencies: #30887

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 7, 2021

Changed commit from 1c6335a to 0c012f7

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 7, 2021

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

5c23cc9DocTestReporter: Fix 'sage -t --optional=all'
1b8634dsage.doctest.external: Add 4ti2
d9d4f99Merge tag '9.4.beta6' into t/30887/public/30887
646e182src/sage/features/four_ti_2.py: Move import of SAGE_ENV inside the `__init__` method, to remove confusion of sage.misc.dev_tools
f0c61f7Merge #30887
b7f7903sage.features.graphviz: Make .name lowercase to match optional tag; refactor through JoinFeature
2c95e70sage.features.pandoc: Make .name lowercase to match optional tag
214fa89sage.features.ffmpeg: Make .name lowercase to match optional tag
eb8f637sage.features.imagemagick: Make .name 'imagemagick' to match optional tag via JoinFeature
0c012f7sage.features.rubiks: Make .name lowercase to match optional tag; refactor through JoinFeature

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 7, 2021

Changed commit from 0c012f7 to 0321b5e

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 7, 2021

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

0321b5esrc/sage/features/rubiks.py: Fixup

@mkoeppe

This comment has been minimized.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 7, 2021

Changed commit from 0321b5e to 1bba486

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 7, 2021

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

1bba486src/sage/features: Change remaining features to make .name equal to the optional tag

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 7, 2021

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

d7bd9c4src/sage/features/gap.py: Set .name of a GapPackage feature as gap_package_PACKAGE

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 7, 2021

Changed commit from 1bba486 to d7bd9c4

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 7, 2021

Changed commit from d7bd9c4 to 9ee3243

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 7, 2021

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

9ee3243Feature: Add documentation

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 7, 2021

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

50248edsage.doctest.external.has_{cplex,gurobi}: Refactor through new Features

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 7, 2021

Changed commit from 9ee3243 to 50248ed

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 7, 2021

Changed author from Matthias Koeppe, ... to Matthias Koeppe

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 8, 2021

Changed dependencies from #30887 to #30887, #32614

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 8, 2021

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

66cf3e1src/sage/doctest/control.py: Fixup handling of sage_optional_tags
1ec0c48src/sage/features/sagemath.py: Add sage.rings.number_field
0063749src/sage/features/sagemath.py: Add features for modules that were optional extensions
14fd1e5src/doc/en/developer/coding_basics.rst: Update discussion of feature tags
27c53acsrc/sage/features/sagemath.py: Add 'sage.plot'
180e31dMerge #30887
10e8d63sage.features.sagemath: Use JoinFeature when tag is different from the actually tested module
654d09csage.features.sagemath: Change sage_optional_tags to sage_features
f63a7d0src/sage/features/: Move features depending on optional packages to separate files
9358502Merge #32614

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 8, 2021

Changed commit from 50248ed to 9358502

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 8, 2021

Changed commit from 9358502 to 6457ee9

@seblabbe
Copy link
Contributor

comment:38

Looks good now, that is, I get All tests passed with sage -btp --optional=sage,optional,external,build src/sage/features/ now.

@seblabbe
Copy link
Contributor

Changed reviewer from Kwankyu Lee, Matthias Koeppe to Kwankyu Lee, Matthias Koeppe, Sébastien Labbé

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 15, 2021

comment:39

Thanks for reviewing!

@seblabbe
Copy link
Contributor

comment:40

Oups, is it too late to put it back to "needs work"? Because patchbot pyflakes says:

src/sage/doctest/external.py:38:1 'urllib.error' imported but unused
src/sage/doctest/external.py:39:1 'urllib.request.Request' imported but unused
src/sage/doctest/external.py:39:1 'urllib.request.urlopen' imported but unused
src/sage/doctest/external.py:40:1 'ssl.SSLContext' imported but unused

src/sage/features/graphviz.py:14:1 '.Feature' imported but unused
src/sage/features/graphviz.py:14:1 '.FeatureTestResult' imported but unused

src/sage/features/latte.py:5:1 '.Feature' imported but unused
src/sage/features/latte.py:5:1 '.FeatureTestResult' imported but unused

src/sage/features/rubiks.py:12:1 '.Feature' imported but unused
src/sage/features/rubiks.py:12:1 '.FeatureTestResult' imported but unused

found 10 pyflakes errors in the modified files
pyflakes -- 0 seconds

Also

========== coverage ==========
git checkout patchbot/ticket_merged
Full doctests in features/internet.py: 2 / 2 = 100%
Missing doctests in features/mcqd.py: 0 / 1 = 0%
Missing doctests in features/meataxe.py: 0 / 1 = 0%
Missing doctests in features/mip_backends.py: 0 / 4 = 0%
Missing doctests in features/sagemath.py: 1 / 7 = 14%
Missing doctests in features/tdlib.py: 0 / 1 = 0%
Coverage went from 50872 / 52638 = 96.645% to 50873 / 52652 = 96.621%
====================

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 15, 2021

Changed commit from 08248a1 to 759c88b

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 15, 2021

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

759c88bsage.doctest, sage.control: Remove unused imports

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 15, 2021

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

74e5c9csrc/sage/features/mcqd.py: Add doctests

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 15, 2021

Changed commit from 759c88b to 74e5c9c

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 15, 2021

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

15729dcsrc/sage/features/mcqd.py: Add doctests

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 15, 2021

Changed commit from 74e5c9c to 15729dc

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 15, 2021

comment:45

Here are some fixes for these style warnings, contributions welcome

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 16, 2021

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

1d02bd0More doctests for coverage

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 16, 2021

Changed commit from 15729dc to 1d02bd0

@kwankyu
Copy link
Collaborator

kwankyu commented Oct 16, 2021

comment:47

Added more doctests to make bots happy. I could not run tests for numerical backends (cplex, gurobi, coin) because I failed to install them.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 16, 2021

comment:48

Thank you! I'll test it with the coin backend.

@vbraun
Copy link
Member

vbraun commented Oct 20, 2021

Changed branch from public/32649 to 1d02bd0

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Nov 13, 2021

Changed commit from 1d02bd0 to none

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

4 participants