`sage.rings.function_field`: Modularization fixes #35230

mkoeppe · 2023-03-05T07:26:34Z

📚 Description

We split out .derivations (which uses modules) from .maps (which doesn't).
We separate the implementation of rational function fields (which do not need Singular for Gröbner basis calculations) from the implementation of more complicated function fields (which do).
We move some imports into methods.

This is for modularization purposes:

Part of Meta-ticket: Modularize sagelib into separate distributions (pip-installable packages) sagemath-... for Sage 10.x #29705

📝 Checklist

I have made sure that the title is self-explanatory and the description concisely explains the PR.
I have linked an issue or discussion.
I have created tests covering the changes.
I have updated the documentation accordingly.

⌛ Dependencies

Depends on sage.features: Add sage.libs.singular, features for standard Python packages #35237 (for the feature sage.libs.singular)
Depends on Deprecate is_FiniteField etc., make sage.rings.finite_rings a namespace package #35119 (to preempt merge conflicts)

codecov-commenter · 2023-03-05T10:32:22Z

Codecov Report

Patch coverage: 91.76% and project coverage change: -0.07 ⚠️

Comparison is base (f449b14) 88.62% compared to head (63be8bf) 88.56%.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #35230      +/-   ##
===========================================
- Coverage    88.62%   88.56%   -0.07%     
===========================================
  Files         2148     2161      +13     
  Lines       398653   398744      +91     
===========================================
- Hits        353308   353129     -179     
- Misses       45345    45615     +270

Impacted Files	Coverage Δ
src/sage/categories/function_fields.py	`87.50% <ø> (ø)`
src/sage/features/__init__.py	`95.18% <ø> (ø)`
src/sage/features/all.py	`100.00% <ø> (ø)`
src/sage/features/bliss.py	`100.00% <ø> (ø)`
src/sage/features/cddlib.py	`100.00% <ø> (ø)`
src/sage/features/csdp.py	`40.74% <ø> (ø)`
src/sage/features/cython.py	`100.00% <ø> (ø)`
src/sage/features/databases.py	`96.15% <ø> (-0.15%)`	⬇️
src/sage/features/dvipng.py	`100.00% <ø> (ø)`
src/sage/features/ffmpeg.py	`22.58% <ø> (-2.42%)`	⬇️
... and 92 more

... and 23 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

…date=format:%Y FILE | sort -k2"

kwankyu · 2023-03-10T02:08:30Z

This PR makes me think whether the direction and the level of the modularization efforts is reasonable.

The branch adds one of # optional - M tags to every doctests depending on the base fields over which function fields are defined, where M is one of sage.libs.singular, sage.libs.pari, and sage.rings.number_field, and also sage.modules for derivations. In the future, no doctest in the function fields module will go without one of these tags since a function field needs a base field. This is very fine modularization that I didn't expect.

If the function fields module is dependent on all of these Ms, we do not need these tags, and one can add a new doctest without worrying which one of those tags should be added to the doctest.

So a basic question is whether some users of the function fields module will need only, say, finite fields as their base fields and want to install only sage.libs.singular along with the function fields module, and thus he/she can save some of disk space and install time. I guess there can be such users for some critical mission, but most users (including me) will be satisfied with time and space saved by much coarse modularization like, say, sage with full function fields module but without manifolds and optimization modules.

If we can still choose the level of the whole modularization efforts, I think we need to decide this on sage-devel, once and for all, before proceeding.

mkoeppe · 2023-03-10T03:19:30Z

In the future, no doctest in the function fields module will go without one of these tags since a function field needs a base field.

No, that's not true. The basic rings ZZ and QQ are always available.

kwankyu · 2023-03-10T03:27:15Z

Ah, I see. If the function field is an extension of rational function field over QQ, the doctest needs sage.libs.singular. So to know which tag is needed, one needs to know the implementation detail.

mkoeppe · 2023-03-10T03:31:13Z

Yes, these dependencies are not always obvious. To some extent these optional tags provide the necessary information for users who need to deploy the modularized distributions.
I would also expect that there may be a few ways how the hard dependencies on some of the libraries can be avoided. For example, Singular is involved whenever a Gröbner basis must be computed. But it's called even for very very simple inputs.

mkoeppe · 2023-03-10T03:33:01Z

For modules where all doctests depend on a feature, I have been using the file-level annotation. # sage.doctest: optional - M.
But most modules mix tests that require no additional features with some that do require additional features.

kwankyu · 2023-03-10T03:39:05Z

All function fields, which are not simple rational function fields, use multivariate polynomial rings, which is provided by Singular. For a user of the function fields module, Singular is a prerequisite, not an additional feature.

mkoeppe · 2023-03-10T03:48:02Z

That's true, currently; but soon we will be able to provide multivariate polynomials using FLINT instead.

kwankyu · 2023-03-10T03:50:52Z

Then those optional tags will need to be updated? This is a drawback of technical dependency added to doctests?

mkoeppe · 2023-03-10T03:58:13Z

Yes, when the underlying library for a functionality changes, some of these annotations will need updating.

Currently for example, the tag sage.libs.pari is on all tests that construct a finite field; but it is also on all tests that require factoring & prime number ranges. I haven't attempted to distinguish these two cases [there are probably more] using semantically higher level tags.

On the other hand, a tag like sage.rings.number_field will not need to be updated when we reimplement number fields.

kwankyu · 2023-03-10T04:08:39Z

Back to my main point: We can just make the distribution package that installs the function fields module dependent on those technical features. Why not? The same question will be raised for every high level mathematical module when most of its doctests would be laced with lots of repetitive optional tags.

mkoeppe · 2023-03-10T04:10:03Z

Why not?

Because the ordinary rational functions also go through the same code, and they are a crucial part of setting up the categories for the basic rings.

mkoeppe · 2023-03-10T04:16:14Z

In some cases, after a mild reorganization it may be possible to replace lots of # optional tags with a file-level annotation.
For example FunctionFieldMaximalOrderInfinite_polymod currently only has doctests using finite fields, not rationals [I have not checked whether that's by design]. So if this were moved to a separate file, we would have much fewer tags.

kwankyu · 2023-03-10T04:17:25Z

It would be possible to separate rational functions code from the extensions (of rational function fields) code. Then rational functions module could be included in a small distribution, and the extensions module would go into a bigger distribution.

kwankyu · 2023-03-19T13:01:15Z

I added missing titles to new files in the PR to your branch

Add missing titles to new files

github-actions · 2023-03-19T17:54:20Z

Documentation preview for this PR is ready! 🎉
Built with commit: 63be8bf

kwankyu · 2023-03-19T20:21:03Z

Nice. Thanks.

mkoeppe · 2023-03-19T21:25:13Z

Thank you!

vbraun · 2023-03-26T12:59:13Z

merge conflict

SageMath version 10.0.beta6, Release Date: 2023-03-26

mkoeppe · 2023-03-26T18:46:13Z

Trivial merge, back to positive

vbraun · 2023-03-29T21:12:30Z

sage -t --long --warn-long 73.8 --random-seed=123 src/sage/interfaces/r.py  # 3 doctests failed

gh-35314: `sage.schemes`: Reformat doctests, add `# optional` annotations  ### 📚 Description  Adding doctest tags `# optional - sage.rings.finite_rings`, `...number_field`, `...padics`. While going through the doctests line by line, I also made the following changes: - some coding style fixes in the doctests (such as adding spaces around some operators and after commas, following PEP 8) - improved the readability of them in the HTML format by breaking long lines to avoid having to scroll horizontally - improved indentation of input and output, in particular when morphisms are displayed - made better use of the horizontal space in the doctests of some modules in `sage.schemes.toric`, which were formatted for a very narrow layout - improved the sphinx markup of some docstrings.  The doctest tags are preparation for being able to test parts of `sage.schemes` in a modularized setting, in which not all rings are available. See https://doc.sagemath.org/html/en/developer/packaging_sage _library.html#doctest-only-dependencies  Part of: - #29705 ### 📝 Checklist    - [x] I have made sure that the title is self-explanatory and the description concisely explains the PR. - [x] I have linked an issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies   - Depends on #35230 (not merged here) for the definition of the feature `sage.rings.finite_rings`. The mass edits adding `# optional` tags were made using the following Emacs macro. ```elisp (defun sage-copy-optional-annotation () "In a 'sage: ' line of a docstring, copy '# optional' from a previous line and advance to the end of the next 'sage: ' line or to the end of the current docstring. If invoked elsewhere, just advance to the end of the next 'sage: ' line." (interactive) (if (save-excursion (beginning-of-line) (looking-at " *sage:")) (let ((tab-stop-list (list (save-excursion (previous-line) (end-of-line) (re-search-backward "# *optional.*$") (- (match-beginning 0) (save-excursion (beginning-of-line) (point))))))) (end-of-line) (just-one-space) (tab-to-tab-stop) (insert (match-string-no-properties 0)) (re-search-forward "sage: \\|\"\"\"") (end-of-line)) (re-search-forward "sage:") (end-of-line))) (with-eval-after-load "python-mode" (define-key python-mode-map (kbd "C-M-;") 'sage-copy-optional- annotation)) ``` URL: #35314 Reported by: Matthias Köppe Reviewer(s): Kwankyu Lee, Matthias Köppe

saraedum · 2023-06-18T21:02:04Z

@kwankyu many of the copyright headers here now only mention you as the copyright holder. I think that's not accurate and all the authors of the files from which the code has been copied should be mentioned.

kwankyu · 2023-06-18T22:31:29Z

Right. Sorry about that. I prepared a PR to fix the mistakes.

mkoeppe · 2023-06-18T23:11:07Z

Also apologies from me – apparently I didn't catch it when I reviewed @kwankyu's contributions to this PR

mkoeppe added c: algebra t: refactoring labels Mar 5, 2023

mkoeppe self-assigned this Mar 5, 2023

mkoeppe added the s: needs review label Mar 5, 2023

mkoeppe added s: needs work and removed s: needs review labels Mar 5, 2023

Matthias Koeppe added 5 commits March 5, 2023 15:04

sage.features: Add 'sage.libs.singular'

b7a0325

sage.features: Add 'sage.libs.singular' (fixup)

6557578

src/sage/features/singular.py: Fix up doctest

8f05e2c

sage.features.standard: New

ebac0f2

src/sage/features/standard.py: Fix up, rename features to match spkgs

a9fc02e

mkoeppe added s: needs review and removed s: needs work labels Mar 6, 2023

Matthias Koeppe added 3 commits March 5, 2023 23:33

src/sage/features/standard.py: Fix up

2fafd82

src/sage/features: Add/update copyright according to "git blame -w --…

86a2c57

…date=format:%Y FILE | sort -k2"

sage.features: Add feature sage.libs.gap

2f0583c

Merge pull request #3 from kwankyu/refactor_function_field

63be8bf

Add missing titles to new files

mkoeppe added s: needs review and removed s: needs work labels Mar 19, 2023

kwankyu added s: positive review and removed s: needs review labels Mar 19, 2023

vbraun removed the s: positive review label Mar 26, 2023

Merge tag '10.0.beta6' into refactor_function_field

5d9a148

SageMath version 10.0.beta6, Release Date: 2023-03-26

mkoeppe added the s: positive review label Mar 26, 2023

src/sage/rings/function_field: Fix linter

bbc1997

vbraun removed the s: positive review label Mar 29, 2023

Matthias Koeppe added 3 commits March 30, 2023 08:46

Merge remote-tracking branch 'upstream/develop' into features

a25f3d9

src/sage/features/standard.py: Remove rpy2 feature for now

9990302

Merge branch 'features' into refactor_function_field

db2f8dd

mkoeppe added the s: positive review label Mar 30, 2023

vbraun merged commit c1a0323 into sagemath:develop Apr 1, 2023

mkoeppe added this to the sage-10.0 milestone Apr 1, 2023

mkoeppe removed the s: positive review label Apr 1, 2023

kwankyu mentioned this pull request Jun 18, 2023

Fix copyright headers in function fields #35795

Merged

5 tasks

mkoeppe deleted the refactor_function_field branch June 18, 2023 23:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`sage.rings.function_field`: Modularization fixes #35230

`sage.rings.function_field`: Modularization fixes #35230

mkoeppe commented Mar 5, 2023 •

edited

Loading

codecov-commenter commented Mar 5, 2023 •

edited

Loading

kwankyu commented Mar 10, 2023 •

edited

Loading

mkoeppe commented Mar 10, 2023

kwankyu commented Mar 10, 2023 •

edited

Loading

mkoeppe commented Mar 10, 2023

mkoeppe commented Mar 10, 2023

kwankyu commented Mar 10, 2023 •

edited

Loading

mkoeppe commented Mar 10, 2023

kwankyu commented Mar 10, 2023 •

edited

Loading

mkoeppe commented Mar 10, 2023

kwankyu commented Mar 10, 2023

mkoeppe commented Mar 10, 2023 •

edited

Loading

mkoeppe commented Mar 10, 2023 •

edited

Loading

kwankyu commented Mar 10, 2023

kwankyu commented Mar 19, 2023

github-actions bot commented Mar 19, 2023

kwankyu commented Mar 19, 2023

mkoeppe commented Mar 19, 2023

vbraun commented Mar 26, 2023

mkoeppe commented Mar 26, 2023

vbraun commented Mar 29, 2023

saraedum commented Jun 18, 2023

kwankyu commented Jun 18, 2023

mkoeppe commented Jun 18, 2023

sage.rings.function_field: Modularization fixes #35230

sage.rings.function_field: Modularization fixes #35230

Conversation

mkoeppe commented Mar 5, 2023 • edited Loading

📚 Description

📝 Checklist

⌛ Dependencies

codecov-commenter commented Mar 5, 2023 • edited Loading

Codecov Report

kwankyu commented Mar 10, 2023 • edited Loading

mkoeppe commented Mar 10, 2023

kwankyu commented Mar 10, 2023 • edited Loading

mkoeppe commented Mar 10, 2023

mkoeppe commented Mar 10, 2023

kwankyu commented Mar 10, 2023 • edited Loading

mkoeppe commented Mar 10, 2023

kwankyu commented Mar 10, 2023 • edited Loading

mkoeppe commented Mar 10, 2023

kwankyu commented Mar 10, 2023

mkoeppe commented Mar 10, 2023 • edited Loading

mkoeppe commented Mar 10, 2023 • edited Loading

kwankyu commented Mar 10, 2023

kwankyu commented Mar 19, 2023

github-actions bot commented Mar 19, 2023

kwankyu commented Mar 19, 2023

mkoeppe commented Mar 19, 2023

vbraun commented Mar 26, 2023

mkoeppe commented Mar 26, 2023

vbraun commented Mar 29, 2023

saraedum commented Jun 18, 2023

kwankyu commented Jun 18, 2023

mkoeppe commented Jun 18, 2023

`sage.rings.function_field`: Modularization fixes #35230

`sage.rings.function_field`: Modularization fixes #35230

mkoeppe commented Mar 5, 2023 •

edited

Loading

codecov-commenter commented Mar 5, 2023 •

edited

Loading

kwankyu commented Mar 10, 2023 •

edited

Loading

kwankyu commented Mar 10, 2023 •

edited

Loading

kwankyu commented Mar 10, 2023 •

edited

Loading

kwankyu commented Mar 10, 2023 •

edited

Loading

mkoeppe commented Mar 10, 2023 •

edited

Loading

mkoeppe commented Mar 10, 2023 •

edited

Loading