Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

QQbar doesn't use canonical coercion for comparison #7984

Closed
robertwb opened this issue Jan 18, 2010 · 9 comments
Closed

QQbar doesn't use canonical coercion for comparison #7984

robertwb opened this issue Jan 18, 2010 · 9 comments
Assignees
@aghitza
Labels
c: basic arithmetic p: major / 3 t: bug
Milestone
sage-4.3.4

Comments

@robertwb
Copy link
Contributor 

sage: QQbar(2) == GF(7)(2)
BOOM

Should be False.

Component: basic arithmetic

Keywords: QQbar comparison

Author: Robert Bradshaw

Reviewer: John Cremona

Merged: sage-4.3.4.alpha0

Issue created by migration from https://trac.sagemath.org/ticket/7984
@robertwb robertwb added this to the sage-4.3.4 milestone Jan 18, 2010
@robertwb
Copy link
Contributor Author

Attachment: 7984-qqbar-cmp.patch.gz

@wjp
Copy link
Mannequin

wjp mannequin commented Jan 18, 2010

comment:1

This patch depends on #4621 but seems to break the doctest added by that patch.

@wjp wjp mannequin added the s: needs work label Jan 18, 2010
@robertwb
Copy link
Contributor Author

comment:2

This patch is correct, the one at #4621 is wrong.

@JohnCremona
Copy link
Member

Author: Robert Bradshaw

@JohnCremona
Copy link
Member

comment:3

This one looks good to me. However, when testing #4621 I noticed that this:

F.<a>= NumberField(x^2-2)
RR(a)

causes an infinite recursion, which is not good, but not caused by this patch.

I am giving this a positive review, and letting #4621 be sorted out after, also the issue above. Perhaps rwb would like to open a ticket for that unless it is already covered by one?

@JohnCremona
Copy link
Member

Changed keywords from none to QQbar comparison

@JohnCremona
Copy link
Member

Reviewer: John Cremona

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Mar 2, 2010

comment:4

Robert: I have merged 7984-qqbar-cmp.patch in your name with a sensible commit message.

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Mar 2, 2010

Merged: sage-4.3.4.alpha0

@sagetrac-mvngu sagetrac-mvngu mannequin removed the s: positive review label Mar 2, 2010
@sagetrac-mvngu sagetrac-mvngu mannequin closed this as completed Mar 2, 2010
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@aghitza aghitza

Labels
c: basic arithmetic p: major / 3 t: bug
Projects
None yet
Milestone
sage-4.3.4
Development

No branches or pull requests
3 participants
@robertwb @aghitza @JohnCremona