Opened 9 months ago

Closed 9 months ago

#32614 closed enhancement (fixed)

Features and optional tags for sage modules provided by separate distributions

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-9.5
Component: doctest framework Keywords:
Cc: gh-kliem, jhpalmieri Merged in:
Authors: Matthias Koeppe Reviewers: John Palmieri, Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 4558791 (Commits, GitHub, GitLab) Commit: 4558791ccdef49ec5045072fabf2c8d9358f066f
Dependencies: Stopgaps:

Status badges

Description (last modified by mkoeppe)

... 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.

Change History (31)

comment:1 Changed 9 months ago by mkoeppe

  • Branch set to u/mkoeppe/features_and_optional_tags_for_sage_subset_distributions

comment:2 Changed 9 months ago by mkoeppe

  • Commit set to 9096edb73bfa435e242635b5d5eb01b62aadd21a

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

comment:3 Changed 9 months ago by git

  • Commit changed from 9096edb73bfa435e242635b5d5eb01b62aadd21a to 747b458420bffc389fdc1e3a1acec4148e818ba4

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

comment:4 Changed 9 months ago by mkoeppe

  • Authors set to Matthias Koeppe
  • Description modified (diff)
  • Summary changed from Features and optional tags for sage subset distributions to Features and optional tags for sage modules provided by separate distributions

comment:5 Changed 9 months ago by mkoeppe

  • Description modified (diff)

comment:6 Changed 9 months ago by git

  • Commit changed from 747b458420bffc389fdc1e3a1acec4148e818ba4 to 5e04a85c4dbe3663bab664b7356d255b5ddc1cf6

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

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

comment:7 Changed 9 months ago by git

  • Commit changed from 5e04a85c4dbe3663bab664b7356d255b5ddc1cf6 to 66cf3e12e49e6d5b86bd6023edc2d9d92ee2b00e

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

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

comment:8 Changed 9 months ago by git

  • Commit changed from 66cf3e12e49e6d5b86bd6023edc2d9d92ee2b00e to 1ec0c485ccaed52ce83ba66c479fadfa3ef9b167

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

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

comment:9 Changed 9 months ago by mkoeppe

  • Status changed from new to needs_review

comment:10 Changed 9 months ago by git

  • Commit changed from 1ec0c485ccaed52ce83ba66c479fadfa3ef9b167 to 006374980e227e2e15b9324d3a3b47b2290d7570

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

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

comment:11 follow-ups: Changed 9 months ago by tscrim

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?

comment:12 in reply to: ↑ 11 Changed 9 months ago by tscrim

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.

comment:13 in reply to: ↑ 11 Changed 9 months ago by mkoeppe

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

comment:14 follow-ups: Changed 9 months ago by jhpalmieri

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.

comment:15 in reply to: ↑ 14 Changed 9 months ago by mkoeppe

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).

comment:16 in reply to: ↑ 14 Changed 9 months ago by mkoeppe

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.*[.]'

comment:17 Changed 9 months ago by mkoeppe

I'll update the documentation

comment:18 Changed 9 months ago by git

  • Commit changed from 006374980e227e2e15b9324d3a3b47b2290d7570 to 14fd1e54a13b1df7f81858019b31fb92cdf36007

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

14fd1e5src/doc/en/developer/coding_basics.rst: Update discussion of feature tags

comment:19 Changed 9 months ago by git

  • Commit changed from 14fd1e54a13b1df7f81858019b31fb92cdf36007 to 27c53ac6450f789246b1df895bc26950a0cceb65

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

27c53acsrc/sage/features/sagemath.py: Add 'sage.plot'

comment:20 Changed 9 months ago by mkoeppe

  • Dependencies set to #30887

comment:21 Changed 9 months ago by git

  • Commit changed from 27c53ac6450f789246b1df895bc26950a0cceb65 to 10e8d6395e1c829b6a28750d3035de95862230b8

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

comment:22 Changed 9 months ago by mkoeppe

  • Description modified (diff)

comment:23 Changed 9 months ago by git

  • Commit changed from 10e8d6395e1c829b6a28750d3035de95862230b8 to 654d09ca4c22f4888d8e511789e57a7627043492

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

654d09csage.features.sagemath: Change sage_optional_tags to sage_features

comment:24 Changed 9 months ago by git

  • Commit changed from 654d09ca4c22f4888d8e511789e57a7627043492 to f63a7d0171ba5f753d1755e0aff3fd00e373590c

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

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

comment:25 Changed 9 months ago by tscrim

  • Reviewers set to John Palmieri, Travis Scrimshaw

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

comment:26 Changed 9 months ago by jhpalmieri

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

comment:27 Changed 9 months ago by tscrim

  • Status changed from needs_review to positive_review

comment:28 Changed 9 months ago by mkoeppe

Thanks for the review!

comment:29 Changed 9 months ago by git

  • Commit changed from f63a7d0171ba5f753d1755e0aff3fd00e373590c to 4558791ccdef49ec5045072fabf2c8d9358f066f
  • Status changed from positive_review to needs_review

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

comment:30 Changed 9 months ago by mkoeppe

  • Dependencies #30887 deleted
  • Status changed from needs_review to positive_review

comment:31 Changed 9 months ago by vbraun

  • Branch changed from u/mkoeppe/features_and_optional_tags_for_sage_subset_distributions to 4558791ccdef49ec5045072fabf2c8d9358f066f
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.