Opened 4 years ago
Last modified 8 days ago
#24209 needs_review defect
Make `I in QQbar` hold
Reported by:  tmonteil  Owned by:  

Priority:  minor  Milestone:  sage9.4 
Component:  coercion  Keywords:  
Cc:  Merged in:  
Authors:  Thierry Monteil  Reviewers:  Samuel Lelièvre 
Report Upstream:  N/A  Work issues:  
Branch:  u/tmonteil/i_in_qqbar_returns_false (Commits, GitHub, GitLab)  Commit:  7144ec2b728f878d0a09cf8ec5b541f4459445ca 
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 (12)
comment:1 Changed 3 years ago by
comment:2 Changed 3 years ago by
comment:3 Changed 5 weeks ago by
 Branch set to u/tmonteil/i_in_qqbar_returns_false
comment:4 Changed 5 weeks 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 5 weeks ago by
 Status changed from needs_review to needs_work
Need another doctest for sqrt(1)
which belongs to SR
.
comment:6 Changed 5 weeks 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 5 weeks ago by
 Status changed from needs_work to needs_review
comment:8 Changed 5 weeks ago by
 Milestone changed from sage9.3 to sage9.4
 Priority changed from critical to minor
comment:9 Changed 5 weeks 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 9 days ago by
 Status changed from positive_review to needs_work
Merge conflict
comment:11 Changed 8 days 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 8 days ago by
 Status changed from needs_work to needs_review
Probably a duplicate of #17984. You might want to voice your opinion there.