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:

Status badges

Description (last modified by slelievre)

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 rws

Probably a duplicate of #17984. You might want to voice your opinion there.

comment:2 Changed 4 years ago by mmezzarobba

The proper way to fix this issue would be via #12715 and #18036. There has been some progress in this direction, but we're not quite there yet.

comment:3 Changed 11 months ago by tmonteil

  • Branch set to u/tmonteil/i_in_qqbar_returns_false

comment:4 Changed 11 months ago by tmonteil

  • Authors set to Thierry Monteil
  • 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 tmonteil

  • 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 git

  • 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 tmonteil

  • Status changed from needs_work to needs_review

comment:8 Changed 11 months ago by mkoeppe

  • Milestone changed from sage-9.3 to sage-9.4
  • Priority changed from critical to minor

comment:9 Changed 11 months ago by slelievre

  • 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: Changed 10 months ago by vbraun

  • Status changed from positive_review to needs_work

Merge conflict

comment:11 Changed 10 months ago by git

  • 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 tmonteil

  • Status changed from needs_work to needs_review

Replying to vbraun:

Merge conflict

Done.

comment:13 Changed 9 months ago by mjo

  • 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 slelievre

  • 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 mkoeppe

  • Milestone changed from sage-9.4 to sage-9.5

comment:16 Changed 8 months ago by vbraun

  • Branch changed from u/slelievre/24209 to 608c5ee5270089e8e91997f9f2be33b6f95df627
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.