Skip to content

Conversation

grhkm21
Copy link
Contributor

@grhkm21 grhkm21 commented Feb 12, 2024

Fix #37296.

Add warnings.filterwarnings to ignore DeprecationWarning in the failing doctest, since it is an unrelated issue and does not affect correctness of the test.

To be more specific, the warning only appears when the rounding error will change, i.e. on $n + \frac{1}{2}$ (I believe). Here are some runs:

sage: a = QQ.random_element(3, 3); _ = round(a)
sage: a = QQ.random_element(3, 3); _ = round(a)
sage: a = QQ.random_element(3, 3); _ = round(a)
sage: a = QQ.random_element(3, 3); _ = round(a)
sage: a = QQ.random_element(3, 3); _ = round(a)
sage: a = QQ.random_element(3, 3); _ = round(a)
<ipython-input-12-179149e36f01>:1: DeprecationWarning: the default rounding for rationals, currently `away`, will be changed to `even`.
See https://github.com/sagemath/sage/issues/35473 for details.
  a = QQ.random_element(Integer(3), Integer(3)); _ = round(a)
sage: a = QQ.random_element(3, 3); _ = round(a)
sage: a = QQ.random_element(3, 3); _ = round(a)
<ipython-input-14-179149e36f01>:1: DeprecationWarning: the default rounding for rationals, currently `away`, will be changed to `even`.
See https://github.com/sagemath/sage/issues/35473 for details.
  a = QQ.random_element(Integer(3), Integer(3)); _ = round(a)
sage: a = QQ.random_element(3, 3); _ = round(a)
sage: a = QQ.random_element(3, 3); _ = round(a)

This is a "correct" fix because the `DeprecationWarning` is unrelated to
the test.
@grhkm21 grhkm21 changed the title Fix #37296 Fix #37296 Random test failures in number_field _element_quadratic Feb 12, 2024
Copy link

Documentation preview for this PR (built with commit fdc7fd7; changes) is ready! 🎉

Copy link
Member

@S17A05 S17A05 left a comment

Choose a reason for hiding this comment

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

LGTM.

@grhkm21
Copy link
Contributor Author

grhkm21 commented Feb 13, 2024

Thanks!

@grhkm21 grhkm21 changed the title Fix #37296 Random test failures in number_field _element_quadratic Fix random test failures in number_field _element_quadratic Feb 13, 2024
@grhkm21 grhkm21 changed the title Fix random test failures in number_field _element_quadratic Fix random test failures in number_field_element_quadratic Feb 13, 2024
vbraun pushed a commit to vbraun/sage that referenced this pull request Feb 18, 2024
sagemathgh-37306: Fix random test failures in `number_field_element_quadratic`
    
Fix sagemath#37296.

Add `warnings.filterwarnings` to ignore `DeprecationWarning` in the
failing doctest, since it is an unrelated issue and does not affect
correctness of the test.

To be more specific, the warning only appears when the rounding error
will change, i.e. on $n + \frac{1}{2}$ (I believe). Here are some runs:

```python
sage: a = QQ.random_element(3, 3); _ = round(a)
sage: a = QQ.random_element(3, 3); _ = round(a)
sage: a = QQ.random_element(3, 3); _ = round(a)
sage: a = QQ.random_element(3, 3); _ = round(a)
sage: a = QQ.random_element(3, 3); _ = round(a)
sage: a = QQ.random_element(3, 3); _ = round(a)
<ipython-input-12-179149e36f01>:1: DeprecationWarning: the default
rounding for rationals, currently `away`, will be changed to `even`.
See sagemath#35473 for details.
  a = QQ.random_element(Integer(3), Integer(3)); _ = round(a)
sage: a = QQ.random_element(3, 3); _ = round(a)
sage: a = QQ.random_element(3, 3); _ = round(a)
<ipython-input-14-179149e36f01>:1: DeprecationWarning: the default
rounding for rationals, currently `away`, will be changed to `even`.
See sagemath#35473 for details.
  a = QQ.random_element(Integer(3), Integer(3)); _ = round(a)
sage: a = QQ.random_element(3, 3); _ = round(a)
sage: a = QQ.random_element(3, 3); _ = round(a)
```
    
URL: sagemath#37306
Reported by: grhkm21
Reviewer(s): Sebastian A. Spindler
vbraun pushed a commit to vbraun/sage that referenced this pull request Feb 19, 2024
sagemathgh-37306: Fix random test failures in `number_field_element_quadratic`
    
Fix sagemath#37296.

Add `warnings.filterwarnings` to ignore `DeprecationWarning` in the
failing doctest, since it is an unrelated issue and does not affect
correctness of the test.

To be more specific, the warning only appears when the rounding error
will change, i.e. on $n + \frac{1}{2}$ (I believe). Here are some runs:

```python
sage: a = QQ.random_element(3, 3); _ = round(a)
sage: a = QQ.random_element(3, 3); _ = round(a)
sage: a = QQ.random_element(3, 3); _ = round(a)
sage: a = QQ.random_element(3, 3); _ = round(a)
sage: a = QQ.random_element(3, 3); _ = round(a)
sage: a = QQ.random_element(3, 3); _ = round(a)
<ipython-input-12-179149e36f01>:1: DeprecationWarning: the default
rounding for rationals, currently `away`, will be changed to `even`.
See sagemath#35473 for details.
  a = QQ.random_element(Integer(3), Integer(3)); _ = round(a)
sage: a = QQ.random_element(3, 3); _ = round(a)
sage: a = QQ.random_element(3, 3); _ = round(a)
<ipython-input-14-179149e36f01>:1: DeprecationWarning: the default
rounding for rationals, currently `away`, will be changed to `even`.
See sagemath#35473 for details.
  a = QQ.random_element(Integer(3), Integer(3)); _ = round(a)
sage: a = QQ.random_element(3, 3); _ = round(a)
sage: a = QQ.random_element(3, 3); _ = round(a)
```
    
URL: sagemath#37306
Reported by: grhkm21
Reviewer(s): Sebastian A. Spindler
@vbraun vbraun merged commit c3e76e1 into sagemath:develop Feb 25, 2024
@grhkm21 grhkm21 deleted the grhkm/issue-37296 branch February 25, 2024 16:56
vbraun pushed a commit to vbraun/sage that referenced this pull request Feb 18, 2025
sagemathgh-39486: Finish changing Rational's round method default rounding to even
    
Follow-up to sagemath#35756 .

(As is, the warning cannot be suppressed)

Related: sagemath#37306,
sagemath#21935

There's a little (large??) problem: previously `K2.<sqrt2> =
QuadraticField(2); K2(9/2).round()` returns `5` **without** a warning
(because sagemath#35756 forget to print the deprecation). Should we add the
deprecation first **then** change this next year?

### 📝 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.
- [x] 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#39486
Reported by: user202729
Reviewer(s): Vincent Neiger
vbraun pushed a commit to vbraun/sage that referenced this pull request Feb 21, 2025
sagemathgh-39486: Finish changing Rational's round method default rounding to even
    
Follow-up to sagemath#35756 .

(As is, the warning cannot be suppressed)

Related: sagemath#37306,
sagemath#21935

There's a little (large??) problem: previously `K2.<sqrt2> =
QuadraticField(2); K2(9/2).round()` returns `5` **without** a warning
(because sagemath#35756 forget to print the deprecation). Should we add the
deprecation first **then** change this next year?

### 📝 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.
- [x] 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#39486
Reported by: user202729
Reviewer(s): Vincent Neiger
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.

Random test failures in number_field_element_quadratic
4 participants