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

sage.features: Add sage.libs.singular, features for standard Python packages #35237

Merged
merged 10 commits into from
Apr 1, 2023

Conversation

mkoeppe
Copy link
Contributor

@mkoeppe mkoeppe commented Mar 6, 2023

📚 Description

This is for the modularization effort, in particular:

We also add missing copyright headers and update existing copyrights in sage.features.

📝 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

@mkoeppe mkoeppe self-assigned this Mar 6, 2023
@codecov-commenter
Copy link

codecov-commenter commented Mar 6, 2023

Codecov Report

Patch coverage: 98.07% and project coverage change: +0.01 🎉

Comparison is base (52a81cb) 88.57% compared to head (2f0583c) 88.58%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #35237      +/-   ##
===========================================
+ Coverage    88.57%   88.58%   +0.01%     
===========================================
  Files         2140     2141       +1     
  Lines       397273   397416     +143     
===========================================
+ Hits        351891   352061     +170     
+ Misses       45382    45355      -27     
Impacted Files Coverage Δ
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%) ⬇️
src/sage/features/gfan.py 100.00% <ø> (ø)
... and 124 more

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 at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@kwankyu
Copy link
Collaborator

kwankyu commented Mar 7, 2023

LGTM.

But why these feature files lack the usual heading of sage files?

@kwankyu
Copy link
Collaborator

kwankyu commented Mar 8, 2023

Thanks. I am waiting for B&T for sanity.

@github-actions
Copy link

github-actions bot commented Mar 8, 2023

Documentation preview for this PR is ready! 🎉
Built with commit: 2f0583c

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 8, 2023

Thank you!

@vbraun
Copy link
Member

vbraun commented Mar 27, 2023

sage -t --long --warn-long 39.2 --random-seed=123 src/sage/interfaces/r.py
**********************************************************************
File "src/sage/interfaces/r.py", line 697, in sage.interfaces.r.R.__reduce__
Failed example:
    rlr, t = r.__reduce__()  # optional - rpy2
Exception raised:
    Traceback (most recent call last):
      File "/home/release/Sage/src/sage/doctest/forker.py", line 695, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/release/Sage/src/sage/doctest/forker.py", line 1093, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.interfaces.r.R.__reduce__[0]>", line 1, in <module>
        rlr, t = r.__reduce__()  # optional - rpy2
                 ^^^^^^^^^^^^^^
      File "/home/release/Sage/local/var/lib/sage/venv-python3.11.1/lib/python3.11/copyreg.py", line 76, in _reduce_ex
        raise TypeError(f"cannot pickle {cls.__name__!r} object")
    TypeError: cannot pickle 'LazyImport' object
**********************************************************************

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 27, 2023

I don't think this is related to the changes here.

Perhaps you installed R on a test machine and this is the first time that rpy2 tests are tested?

@mkoeppe mkoeppe changed the title sage.features: Add sage.libs.singular, features for standard Python packages sage.features: Add sage.libs.singular, features for standard Python packages, activate/fix rpy2 tests Mar 29, 2023
@mkoeppe mkoeppe requested a review from jhpalmieri March 29, 2023 23:57
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 30, 2023

Doctest failures because deprecation warnings for the same lazy_import are now issued separately for each doctest.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 30, 2023

I have deferred the work to fix the rpy2 test failures to #35396.

As a quick fix for this PR, 9990302 just removes the rpy2 feature detection again.

@mkoeppe mkoeppe changed the title sage.features: Add sage.libs.singular, features for standard Python packages, activate/fix rpy2 tests sage.features: Add sage.libs.singular, features for standard Python packages Mar 30, 2023
@vbraun vbraun merged commit c9daf36 into sagemath:develop Apr 1, 2023
vbraun pushed a commit that referenced this pull request Apr 1, 2023
    
<!-- ^^^^^
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 #1234" use "Introduce new method to
calculate 1+1"
-->
### 📚 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 #29705



<!-- Describe your changes here in detail -->
<!-- Why is this change required? What problem does it solve? -->
<!-- If it resolves an open issue, please link to the issue here. For
example "Closes #1337" -->

### 📝 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! -->

- [x] 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
<!-- List all open pull requests that this PR logically depends on -->
<!--
- #xyz: short description why this is a dependency
- #abc: ...
-->
- Depends on #35237 (for the feature `sage.libs.singular`)
- Depends on #35119 (to preempt merge conflicts)
    
URL: #35230
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee, Matthias Köppe
vbraun pushed a commit that referenced this pull request Apr 1, 2023
…tion

    
<!-- ^^^^^
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 #1234" use "Introduce new method to
calculate 1+1"
-->
### 📚 Description

<!-- Describe your changes here in detail -->
<!-- Why is this change required? What problem does it solve? -->
<!-- If it resolves an open issue, please link to the issue here. For
example "Closes #1337" -->
These tags make it possible/meaningful to doctest `sage.graphs` even
when some "standard" packages (`networkx`, `numpy`, the modularized
distribution packaging providing `sage.groups`) are not installed.

- Part of #29705
- Split out from #34998

### 📝 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! -->

- [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
<!-- List all open pull requests that this PR logically depends on -->
<!--
- #xyz: short description why this is a dependency
- #abc: ...
-->
This is just one commit on top of:
- #35237
    
URL: #35266
Reported by: Matthias Köppe
Reviewer(s): David Coudert, Dima Pasechnik, Matthias Köppe
vbraun pushed a commit that referenced this pull request Apr 1, 2023
…ags for modularization

    
<!-- ^^^^^
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 #1234" use "Introduce new method to
calculate 1+1"
-->
### 📚 Description

<!-- Describe your changes here in detail -->
<!-- Why is this change required? What problem does it solve? -->
<!-- If it resolves an open issue, please link to the issue here. For
example "Closes #1337" -->

Adding tags that make it possible/meaningful to doctest
`sage.manifolds`, `sage.tensor` even when some "standard" packages
(`pplpy`, the modularized distribution packaging providing
`sage.symbolic` and `sage.manifolds`) are not installed. Also taking
care of `sage.geometry.riemannian_manifolds`.

- Part of #29705
- Split out from #34998

### 📝 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! -->

- [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
<!-- List all open pull requests that this PR logically depends on -->
<!--
- #xyz: short description why this is a dependency
- #abc: ...
-->
This is just two commits on top of:
- #35237
    
URL: #35267
Reported by: Matthias Köppe
Reviewer(s): Eric Gourgoulhon
@mkoeppe mkoeppe added this to the sage-10.0 milestone Apr 1, 2023
@mkoeppe mkoeppe deleted the features branch April 15, 2023 23:20
vbraun pushed a commit to vbraun/sage that referenced this pull request Jun 10, 2024
    
<!-- 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 sagemath#12345", use "Add a new method to
multiply two integers" -->

### 📚 Description

<!-- Describe your changes here in detail. -->

As a follow-up on:
- sagemath#35237

we make `rpy2` a dynamically detected feature, enabling the automatic
run of doctests tagged `# optional - rpy2`; we use a file-level doctest
tag for the `r.py` file.

We also update the spkg documentation for sagemath#35347

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

Resolves sagemath#35347

<!-- 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.
- [ ] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] 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: ...
-->
- Depends on sagemath#35237
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#35396
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee, Matthias Köppe
vbraun pushed a commit to vbraun/sage that referenced this pull request Jun 16, 2024
    
<!-- 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 sagemath#12345", use "Add a new method to
multiply two integers" -->

### 📚 Description

<!-- Describe your changes here in detail. -->

As a follow-up on:
- sagemath#35237

we make `rpy2` a dynamically detected feature, enabling the automatic
run of doctests tagged `# optional - rpy2`; we use a file-level doctest
tag for the `r.py` file.

We also update the spkg documentation for sagemath#35347

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

Resolves sagemath#35347

<!-- 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.
- [ ] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] 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: ...
-->
- Depends on sagemath#35237
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#35396
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee, Matthias Köppe
vbraun pushed a commit to vbraun/sage that referenced this pull request Jun 16, 2024
    
<!-- 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 sagemath#12345", use "Add a new method to
multiply two integers" -->

### 📚 Description

<!-- Describe your changes here in detail. -->

As a follow-up on:
- sagemath#35237

we make `rpy2` a dynamically detected feature, enabling the automatic
run of doctests tagged `# optional - rpy2`; we use a file-level doctest
tag for the `r.py` file.

We also update the spkg documentation for sagemath#35347

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

Resolves sagemath#35347

<!-- 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.
- [ ] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] 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: ...
-->
- Depends on sagemath#35237
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#35396
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee, Matthias Köppe
vbraun pushed a commit to vbraun/sage that referenced this pull request Jun 17, 2024
    
<!-- 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 sagemath#12345", use "Add a new method to
multiply two integers" -->

### 📚 Description

<!-- Describe your changes here in detail. -->

As a follow-up on:
- sagemath#35237

we make `rpy2` a dynamically detected feature, enabling the automatic
run of doctests tagged `# optional - rpy2`; we use a file-level doctest
tag for the `r.py` file.

We also update the spkg documentation for sagemath#35347

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

Resolves sagemath#35347

<!-- 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.
- [ ] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] 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: ...
-->
- Depends on sagemath#35237
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#35396
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee, Matthias Köppe
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants