Skip to content

Remove outdated needs sage.rings.real_interval_field annotations#41404

Merged
vbraun merged 4 commits intosagemath:developfrom
tobiasdiez:removeOutdatedNeeds
Feb 1, 2026
Merged

Remove outdated needs sage.rings.real_interval_field annotations#41404
vbraun merged 4 commits intosagemath:developfrom
tobiasdiez:removeOutdatedNeeds

Conversation

@tobiasdiez
Copy link
Copy Markdown
Contributor

@tobiasdiez tobiasdiez commented Jan 6, 2026 •
edited
Loading

Fixes #41349. Also a bit of minor cleanup in the touched files, like reordering imports and compatifying if-else constructions.

📝 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

@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 6, 2026 •
edited
Loading

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

@tobiasdiez tobiasdiez requested a review from user202729 January 6, 2026 21:34
tscrim
tscrim approved these changes Jan 9, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@tscrim tscrim left a comment

Choose a reason for hiding this comment

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

Happy to have this PR in.

@tobiasdiez
Copy link
Copy Markdown
Contributor Author

tobiasdiez commented Jan 9, 2026

Thanks!

@vbraun
Copy link
Copy Markdown
Member

vbraun commented Jan 11, 2026

I'm getting

src/bin/sage -t --long --warn-long 30.0 --random-seed=123 src/sage/sets/real_set.py
**********************************************************************
File "src/sage/sets/real_set.py", line 2663, in sage.sets.real_set.?.simplest_rational
Failed example:
    s.simplest_rational()
Expected:
    Traceback (most recent call last):
    ...
    NotImplementedError:
Got:
    <BLANKLINE>
    Traceback (most recent call last):
      File "/home/release/Sage/src/sage/doctest/forker.py", line 734, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/release/Sage/src/sage/doctest/forker.py", line 1158, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.sets.real_set.?.simplest_rational[11]>", line 1, in <module>
        s.simplest_rational()
      File "/home/release/Sage/src/sage/sets/real_set.py", line 2706, in simplest_rational
        raise NotImplementedError
    NotImplementedError
**********************************************************************

tscrim
tscrim reviewed Jan 13, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@tscrim tscrim left a comment

Choose a reason for hiding this comment

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

Very scary; it looks like something is now getting tested that was not being done before. At least the changes required are trivial.

Comment thread src/sage/sets/real_set.py Outdated
Copy link
Copy Markdown
Collaborator

@tscrim tscrim Jan 13, 2026

Choose a reason for hiding this comment

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

Suggested change
NotImplementedError

Comment thread src/sage/sets/real_set.py Outdated
Copy link
Copy Markdown
Collaborator

@tscrim tscrim Jan 13, 2026

Choose a reason for hiding this comment

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

Suggested change
NotImplementedError

Comment thread src/sage/sets/real_set.py Outdated
Copy link
Copy Markdown
Collaborator

@tscrim tscrim Jan 13, 2026

Choose a reason for hiding this comment

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

Suggested change
EmptySetError

vbraun pushed a commit to vbraun/sage that referenced this pull request Jan 30, 2026
69f2ff6 
sagemathgh-41404: Remove outdated `needs sage.rings.real_interval_field ` annotations
    
Fixes sagemath#41349. Also a bit of minor cleanup in the touched files, like
reordering imports and compatifying if-else constructions.

<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->



### 📝 Checklist

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

- [ ] 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

<!-- 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#41404
Reported by: Tobias Diez
Reviewer(s): Travis Scrimshaw
@vbraun vbraun merged commit 55cead3 into sagemath:develop Feb 1, 2026
21 of 23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tscrim tscrim tscrim approved these changes

@user202729 user202729 Awaiting requested review from user202729

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

needs sage.rings.real_interval_field requires nonexistent module

4 participants

@tobiasdiez @vbraun @tscrim @cxzhong