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:  sage9.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 sage8.1 to sage9.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 sage9.3 to sage9.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 followup: ↓ 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 sage9.4 to sage9.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.