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

Features and optional tags for sage modules provided by separate distributions #32614

Closed
mkoeppe opened this issue Oct 2, 2021 · 43 comments
Closed

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 2, 2021

... so that we can start writing # optional - sage.symbolic and similar.

We use it in #32432 (sagemath-polyhedra) to skip doctests that depend on sage.graphs, sage.combinat, sage.rings.number_field etc.

CC: @kliem @jhpalmieri

Component: doctest framework

Author: Matthias Koeppe

Branch/Commit: 4558791

Reviewer: John Palmieri, Travis Scrimshaw

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

@mkoeppe mkoeppe added this to the sage-9.5 milestone Oct 2, 2021
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 2, 2021

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 2, 2021

comment:2

Currently optional tags can contain neither . nor -:

sage:         sage: from sage.doctest.parsing import parse_optional_tags
....: 
sage: parse_optional_tags("sage: #optional -- my.p.kg")
{'my'}
sage: parse_optional_tags("sage: #optional -- sagemath-symbolics")
{'sagemath'}

New commits:

9096edbsage.features.sagemath: New

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 2, 2021

Commit: 9096edb

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 3, 2021

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

2938e13src/sage/doctest/parsing.py: Allow . in optional tags
747b458sage.doctest.control, sage.features.sagemath: Provide/use optional tags

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 3, 2021

Changed commit from 9096edb to 747b458

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 3, 2021

Author: Matthias Koeppe

@mkoeppe

This comment has been minimized.

@mkoeppe mkoeppe changed the title Features and optional tags for sage subset distributions Features and optional tags for sage modules provided by separate distributions Oct 3, 2021
@mkoeppe

This comment has been minimized.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 3, 2021

Changed commit from 747b458 to 5e04a85

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 3, 2021

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

e3a541eFixup
5e04a85src/sage/features/sagemath.py: Fixup

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 3, 2021

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

66cf3e1src/sage/doctest/control.py: Fixup handling of sage_optional_tags

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 3, 2021

Changed commit from 5e04a85 to 66cf3e1

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 3, 2021

Changed commit from 66cf3e1 to 1ec0c48

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 3, 2021

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

1ec0c48src/sage/features/sagemath.py: Add sage.rings.number_field

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 3, 2021

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

0063749src/sage/features/sagemath.py: Add features for modules that were optional extensions

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 3, 2021

Changed commit from 1ec0c48 to 0063749

@tscrim
Copy link
Collaborator

tscrim commented Oct 3, 2021

comment:11

Next round of stupid questions: How is this going to work? Are we doing to have to add many such markers to a lot of doctests? How will we as developers test which ones we need to add?

@tscrim
Copy link
Collaborator

tscrim commented Oct 3, 2021

comment:12

Replying to @tscrim:
Are we doing to have to add many such markers to a lot of doctests?

I guess this part will be alleviated by #30778.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 4, 2021

comment:13

Replying to @tscrim:

How will we as developers test which ones we need to add?

By running the test infrastructure of the subset distributions, such as the one added in #32432 (sagemath-polyhedra). This can be done locally, but we will also run this automatically on GH Actions

@jhpalmieri
Copy link
Member

comment:14

Still no hyphens allowed in # optional - keyword? In any case, please also change the documentation in developer/coding_basics.rst to match the behavior (currently says "Any punctuation (periods, commas, hyphens, semicolons, ...) after the first word ends the list of packages."). I think we should have a clearly defined (and well-thought out, or is that too much to ask?) syntax for the keywords. This section of the documentation should ideally also provide examples in which tags are combined, as in # optional - abc, long time. And I guess that is why we chose to parse # optional - abc, xyz as abc rather than abc and xyz. It is a little odd that with the proposed changes, # optional - abc. long time will be treated very differently than # optional - abc, long time.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 4, 2021

comment:15

Replying to @jhpalmieri:

Still no hyphens allowed in # optional - keyword?

That's right -- this reflects the fact that our tags for optional packages use the spkg names, which use underscore, not dash.
Dashes appear in Python distribution package names (tools like setuptools normalize underscores to dashes).

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 4, 2021

comment:16

Replying to @jhpalmieri:

It is a little odd that with the proposed changes, # optional - abc. long time will be treated very differently than # optional - abc, long time.

Using . like this has not been observed in the wild, as can be checked with git grep '#.*optional.*[.]'

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 4, 2021

comment:17

I'll update the documentation

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 4, 2021

Changed commit from 0063749 to 14fd1e5

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 7, 2021

Dependencies: #30887

@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:

ea548d7sage.features.four_ti_2: New, use it in sage.interfaces.four_ti_2, sage.sandpiles
f826dedbuild/pkgs/4ti2/spkg-configure.m4: Check for executable's with prefix 4ti2_ too
56016ceuse AC_LINK_IFELSE instead of obsolete AC_TRY_LINK
2b45b77sage.feature.join_feature: New, factored out from LatteFeature; use it to implement FourTi2Feature
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
180e31dMerge #30887
10e8d63sage.features.sagemath: Use JoinFeature when tag is different from the actually tested module

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 7, 2021

Changed commit from 27c53ac to 10e8d63

@mkoeppe

This comment has been minimized.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 7, 2021

Changed commit from 10e8d63 to 654d09c

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 7, 2021

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

654d09csage.features.sagemath: Change sage_optional_tags to sage_features

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 7, 2021

Changed commit from 654d09c to f63a7d0

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 7, 2021

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

f63a7d0src/sage/features/: Move features depending on optional packages to separate files

@tscrim
Copy link
Collaborator

tscrim commented Oct 7, 2021

Reviewer: John Palmieri, Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Oct 7, 2021

comment:25

John, any other comments? I think I am ready to set a positive review.

@jhpalmieri
Copy link
Member

comment:26

I am happy with it. Please go ahead with a positive review when you are ready.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 8, 2021

comment:28

Thanks for the review!

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 11, 2021

Changed commit from f63a7d0 to 4558791

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 11, 2021

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

4558791Merge tag '9.5.beta3' into t/32614/features_and_optional_tags_for_sage_subset_distributions

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 11, 2021

Changed dependencies from #30887 to none

@vbraun
Copy link
Member

vbraun commented Oct 13, 2021

@vbraun vbraun closed this as completed in b743b48 Oct 13, 2021
mkoeppe added a commit to mkoeppe/sage that referenced this issue Feb 12, 2023
mkoeppe added a commit to mkoeppe/sage that referenced this issue Feb 12, 2023
mkoeppe added a commit to mkoeppe/sage that referenced this issue Feb 12, 2023
mkoeppe added a commit to mkoeppe/sage that referenced this issue Feb 12, 2023
mkoeppe added a commit to mkoeppe/sage that referenced this issue Feb 17, 2023
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