Skip to content

Conversation

@jeriedel24
Copy link

The correct module path is "sage.rings" instead of "sage.ring".
In sage/modular/pollack_stevens/padic_lseries.py and in sage/structure/factory.pyx the typo prevents doctests from being run because the "needs" requirement canot be fulfilled.
In sage/rings/polynomial/padics/polynomial_padic.py the typo is merely cosmetic.

See brief discussion in Sage Support Group

📝 Checklist

/

  • The title is concise and informative.
  • 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 and checked the documentation preview.

⌛ Dependencies

The correct module path is "sage.rings" instead of "sage.ring".
In sage/modular/pollack_stevens/padic_lseries.py and in
sage/structure/factory.pyx the typo prevents doctests from being run.
In sage/rings/polynomial/padics/polynomial_padic.py the typo is merely
cosmetic.
Copy link
Contributor

@dcoudert dcoudert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@github-actions
Copy link

Documentation preview for this PR (built with commit d08315f; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@jeriedel24
Copy link
Author

The failure occurs while running "sage -t --long src/sage/rings/tests.py".
Since this PR does not change the file src/sage/rings/tests.py, and since it does not change any code that might be imported in that file, I cannot see why this PR could cause the test failure.
On my test system (AlmaLinux 9.4, Python 3.12.1, Sage 10.4.beta9), running a long test on the three files changed in this PR and on src/sage/rings/tests.py simultaneously is successful:

sage -t --long --random-seed=117332830981319924244841299404508004523 src/sage/rings/polynomial/padics/polynomial_padic.py
    [69 tests, 0.59 s]
sage -t --long --random-seed=117332830981319924244841299404508004523 src/sage/structure/factory.pyx
    [118 tests, 8.63 s]
sage -t --long --random-seed=117332830981319924244841299404508004523 src/sage/rings/tests.py
    [62 tests, 74.32 s]
sage -t --long --random-seed=117332830981319924244841299404508004523 src/sage/modular/pollack_stevens/padic_lseries.py
    [89 tests, 1165.14 s]

Perhaps the timeout failure while testing src/sage/rings/tests.py was just by chance, and a re-run of the test might be successful?

vbraun pushed a commit to vbraun/sage that referenced this pull request Jun 16, 2024
sagemathgh-38215: Fix typo "sage.ring" in docstrings
    
The correct module path is "sage.rings" instead of "sage.ring".
In sage/modular/pollack_stevens/padic_lseries.py and in
sage/structure/factory.pyx the typo prevents doctests from being run
because the "needs" requirement canot be fulfilled.
In sage/rings/polynomial/padics/polynomial_padic.py the typo is merely
cosmetic.

See brief [discussion in Sage Support
Group](https://groups.google.com/g/sage-support/c/Msx0QJF7rpA)

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
/

- [x] The title is concise and informative.
- [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.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#38215
Reported by: Jens-Erik Riedel
Reviewer(s): David Coudert, Julian Rüth
vbraun pushed a commit to vbraun/sage that referenced this pull request Jun 16, 2024
sagemathgh-38215: Fix typo "sage.ring" in docstrings
    
The correct module path is "sage.rings" instead of "sage.ring".
In sage/modular/pollack_stevens/padic_lseries.py and in
sage/structure/factory.pyx the typo prevents doctests from being run
because the "needs" requirement canot be fulfilled.
In sage/rings/polynomial/padics/polynomial_padic.py the typo is merely
cosmetic.

See brief [discussion in Sage Support
Group](https://groups.google.com/g/sage-support/c/Msx0QJF7rpA)

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
/

- [x] The title is concise and informative.
- [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.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#38215
Reported by: Jens-Erik Riedel
Reviewer(s): David Coudert, Julian Rüth
@vbraun vbraun merged commit 1fc0dde into sagemath:develop Jun 22, 2024
@jeriedel24 jeriedel24 deleted the fix_ring_in_module_path branch April 8, 2025 10:24
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.

6 participants