Opened 5 years ago
Closed 8 months ago
#24209 closed defect (fixed)
Make `I in QQbar` hold
Reported by: | tmonteil | Owned by: | |
---|---|---|---|
Priority: | minor | Milestone: | sage-9.5 |
Component: | coercion | Keywords: | |
Cc: | mjo | Merged in: | |
Authors: | Thierry Monteil | Reviewers: | Samuel Lelièvre, Michael Orlitzky |
Report Upstream: | N/A | Work issues: | |
Branch: | 608c5ee (Commits, GitHub, GitLab) | Commit: | 608c5ee5270089e8e91997f9f2be33b6f95df627 |
Dependencies: | Stopgaps: |
Description (last modified by )
As reported on Ask Sage question 39509 (I doesn't belong to QQbar, why?):
sage: I in QQbar False
This is because in the __contains__
method of the Parent
class, the big try...except
fails when evaluating the following:
sage: bool(I == QQbar(I)) TypeError: unsupported operand parent(s) for +: 'Number Field in I with defining polynomial x^2 + 1' and 'Algebraic Field'
This early fail does not allow further test to return True
.
Fixed in #31628. We add a doctest here.
Change History (16)
comment:1 Changed 4 years ago by
comment:2 Changed 4 years ago by
comment:3 Changed 11 months ago by
- Branch set to u/tmonteil/i_in_qqbar_returns_false
comment:4 Changed 11 months ago by
- Commit set to 58b14db855d222c92062b1814bdaaada58acea6d
- Milestone changed from sage-8.1 to sage-9.3
- Status changed from new to needs_review
The issue was solved elsewhere, but it is good to prevent future regression with a dedicated doctest.
New commits:
58b14db | #24209 : add doctest for I in QQbar
|
comment:5 Changed 11 months ago by
- Status changed from needs_review to needs_work
Need another doctest for sqrt(-1)
which belongs to SR
.
comment:6 Changed 11 months ago by
- Commit changed from 58b14db855d222c92062b1814bdaaada58acea6d to cb580774d0d27341e864f2e902afec0dd166f2e3
Branch pushed to git repo; I updated commit sha1. New commits:
cb58077 | #24209 : add doctest for sqrt(-1) in QQbar
|
comment:7 Changed 11 months ago by
- Status changed from needs_work to needs_review
comment:8 Changed 11 months ago by
- Milestone changed from sage-9.3 to sage-9.4
- Priority changed from critical to minor
comment:9 Changed 11 months ago by
- Description modified (diff)
- Reviewers set to Samuel Lelièvre
- Status changed from needs_review to positive_review
- Summary changed from I in QQbar returns False to Make `I in QQbar` hold
I believe this was solved by #31628.
comment:10 follow-up: ↓ 12 Changed 10 months ago by
- Status changed from positive_review to needs_work
Merge conflict
comment:11 Changed 10 months ago by
- Commit changed from cb580774d0d27341e864f2e902afec0dd166f2e3 to 7144ec2b728f878d0a09cf8ec5b541f4459445ca
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
7144ec2 | #24209 : add doctest for I in QQbar and sqrt(-1) in QQbar
|
comment:12 in reply to: ↑ 10 Changed 10 months ago by
- Status changed from needs_work to needs_review
comment:13 Changed 9 months ago by
- Cc mjo added
- Reviewers changed from Samuel Lelièvre to Samuel Lelièvre, Michael Orlitzky
- Status changed from needs_review to positive_review
Still fine.
comment:14 Changed 9 months ago by
- Branch changed from u/tmonteil/i_in_qqbar_returns_false to u/slelievre/24209
- Commit changed from 7144ec2b728f878d0a09cf8ec5b541f4459445ca to 608c5ee5270089e8e91997f9f2be33b6f95df627
Rebased on 9.4.rc2 in the hope to help patchbots. Curious why the last four failed.
New commits:
608c5ee | #24209 : add doctest for I in QQbar and sqrt(-1) in QQbar
|
comment:15 Changed 9 months ago by
- Milestone changed from sage-9.4 to sage-9.5
comment:16 Changed 8 months ago by
- Branch changed from u/slelievre/24209 to 608c5ee5270089e8e91997f9f2be33b6f95df627
- Resolution set to fixed
- Status changed from positive_review to closed
Probably a duplicate of #17984. You might want to voice your opinion there.