sage.features: Declare features as "standard" explicitly#35820
sage.features: Declare features as "standard" explicitly#35820vbraun merged 32 commits intosagemath:developfrom
sage.features: Declare features as "standard" explicitly#35820Conversation
… into hide_features_34185
… into hide_features_34185
… into hide_features_34185
… into hide_features_34185
… into hide_features_34185
…rac.sagemath.org:sage into hide_features_34185
… into hide_features_34185
… type if SAGE_ROOT/build/pkgs is available
|
Looks promising. I am not even objecting to the term "butchered" :P even if I would prefer "annihilated". In any case, I should plan work on my own follow up. I think with that and some other stuff, getting rid of |
|
I think features for some optional packages are still missing; there may be some open tickets for some of these. When that is fixed, I'm all in favor of eliminating the use of sage.misc.packages from the doctester! |
|
Great! Thank you for promptly resolving this issue. It seems like a good solution. In the logs of the doctests there are some features with missing |
|
Documentation preview for this PR (built with commit 4eddb7c; changes) is ready! 🎉 |
|
LGTM! |
|
Thanks for reviewing! |
I've opened #35856 to track the progress on that. |
gh-35749: Add style guide / reference for `# optional - sage....` doctest tags, extend `sage -t` and `sage -fixdoctests` for modularization tasks <!-- Please provide a concise, informative and self-explanatory title. --> <!-- Don't put issue numbers in the title. Put it in the Description below. --> <!-- For example, instead of "Fixes #12345", use "Add a new method to multiply two integers" --> ### 📚 Description <!-- Describe your changes here in detail. --> - Fixing the handling of file-level `# optional` tags. - Some files were not being doctested; fixing the recently introduced errors in doctests. - Implementing block-scoped tags (originally done in PR #35779, merged here) - Expanding the documentation on the `# optional` / `# needs` tags used for modularization purposes, with examples. - Adding features `sage.modular`, `sage.numerical.mip`, `sage.rings.complex_double`, `sage.sat` - The tools `sage -t` and `sage --fixdoctests` receive some new options for modularization tasks (see added documentation): ``` $ make pypi-wheels $ make SAGE_CHECK=yes sagemath-modules $ ./sage --fixdoctests --distribution sagemath-modules \ src/sage/combinat/root_system/root_lattice_realization_algebras.py ``` (example uses #35095) - Suppressing `# optional`/`# needs` of present features in the help system . <!-- Why is this change required? What problem does it solve? --> Motivated by the sage-devel thread https://groups.google.com/g/sage- devel/c/utA0N1it0Eo (2023-06) <!-- If this PR resolves an open issue, please link to it here. For example "Fixes #12345". --> - Resolves #35790 - Resolves #35750 - Part of #29705 <!-- If your change requires a documentation PR, please link it appropriately. --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. It should be `[x]` not `[x ]`. --> - [x] The title is concise, informative, and self-explanatory. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [x] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on - #12345: short description why this is a dependency - #34567: ... --> - Depends on #35820 (merged here) - Vote at https://groups.google.com/g/sage-devel/c/8KZNRBs6F6U (result: https://groups.google.com/g/sage-devel/c/MtS2u3VbJEo) <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> URL: #35749 Reported by: Matthias Köppe Reviewer(s): Kwankyu Lee, Matthias Köppe
sagemathgh-36741: Doctest hide option: Better detection of hidden packages <!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes sagemath#1234" use "Introduce new method to calculate 1+1" --> <!-- Describe your changes here in detail --> <!-- Why is this change required? What problem does it solve? --> <!-- If this PR resolves an open issue, please link to it here. For example "Fixes sagemath#12345". --> <!-- If your change requires a documentation PR, please link it appropriately. --> This PR implements the suggestion made in sagemath#36696 (comment). This means that it introduces a counter in the feature class to detect the number of events a feature has been asked to be present while it was hidden. This allows to remove the call of the `is_present` method in `doctest/control.py`. In order to test it I ran `sage -tp --all --hide=all`. Ideally this should give *All tests passed*. But I got two failing files. One of those was `src/sage/features/databases.py` because `database_cremona_mini_ellcurve` was detected to be optional even thought it is a standard package. This is a leftover of sagemath#35820 which I fix here. Similarily I got 37 failures in `src/sage/groups/abelian_gps/abelian_group.py` since `gap_package_polycyclic` was classified optional even though it is a Gap standard package since Gap 4.10 (as well as `gap_package_atlasrep`). But in this case a corresponding fix, i.e.: ``` def all_features(): - return [GapPackage("atlasrep", spkg="gap_packages"), + return [GapPackage("atlasrep", spkg="gap_packages", type='standard'), GapPackage("design", spkg="gap_packages"), GapPackage("grape", spkg="gap_packages"), GapPackage("guava", spkg="gap_packages"), GapPackage("hap", spkg="gap_packages"), - GapPackage("polycyclic", spkg="gap_packages"), + GapPackage("polycyclic", spkg="gap_packages", type='standard'), GapPackage("qpa", spkg="gap_packages"), GapPackage("quagroup", spkg="gap_packages")] ``` is not the correct way (it gives `UserWarning: Feature gap_package_polycyclic is declared standard, but it is provided by gap_packages, which is declared optional in SAGE_ROOT/build/pkgs`). I would say, the correct fix would be to remove both Gap packages from the `all_features` list and erase the corresponding *optional tags* in `permgroup_named.py`, `distance_regular.pyx`, `abelian_aut.py`, `abelian_group.py` and `abelian_group_gap.py`. If you agree I will open another PR for this. BTW: there seem to be more packages with ambiguous type (detected with current Docker image): ``` Digest: sha256:790197bb223bd7e20b0a2e055aa7b4c5846dc86d94b2425cd233cb6160a5b944 Status: Downloaded newer image for sagemath/sagemath:develop ┌────────────────────────────────────────────────────────────────────┐ │ SageMath version 10.2.rc4, Release Date: 2023-11-17 │ │ Using Python 3.11.1. Type "help()" for help. │ └────────────────────────────────────────────────────────────────────┘ ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ ┃ Warning: this is a prerelease version, and it may be unstable. ┃ ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ sage: from sage.features.all import all_features sage: [(f, f._spkg_type()) for f in all_features() if f.is_present() and f._spkg_type() != 'standard'] [(Feature('sage.misc.cython'), 'optional'), (Feature('database_cremona_mini_ellcurve': Cremona's database of elliptic curves), 'optional'), (Feature('gap_package_atlasrep'), 'optional'), (Feature('gap_package_polycyclic'), 'optional'), (Feature('sage.libs.ecl'), 'optional'), (Feature('sage.libs.gap'), 'optional'), (Feature('sage.libs.singular'), 'optional'), (Feature('sage.numerical.mip'), 'optional')] sage: sage: [(f, f._spkg_type()) for f in all_features() if not f.is_present() and f._spkg_type() == 'standard'] [(Feature('sagemath_doc_html'), 'standard'), (Feature('sage.sat'), 'standard')] ``` ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> <!-- Feel free to remove irrelevant items. --> - [x] The title is concise, informative, and self-explanatory. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [x] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on - sagemath#12345: short description why this is a dependency - sagemath#34567: ... --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> URL: sagemath#36741 Reported by: Sebastian Oehms Reviewer(s): Matthias Köppe, Sebastian Oehms
📚 Description
As discussed in #35668 (comment)
📝 Checklist
⌛ Dependencies