#31551 closed defect (fixed)

Incorrect conversion from ℚ[-i] to SR

Reported by: Marc Mezzarobba Owned by:
Priority: blocker Milestone: sage-9.3
Component: symbolics Keywords:
Cc: Vincent Delecroix Merged in:
Authors: Marc Mezzarobba Reviewers: Vincent Delecroix
Report Upstream: N/A Work issues:
Branch: fddaa2c (Commits, GitHub, GitLab) Commit: fddaa2c0b55dffaa4d3b1c616d291da18c583d98
Dependencies: Stopgaps:

Status badges

Description (last modified by Marc Mezzarobba)

This should return -I, not I:

sage: K.<j> = QuadraticField(-1, embedding=CC(0,-1))
sage: SR(j)
I

Works under Sage 9.2, so maybe related to #18036?

Change History (11)

comment:1 Changed 18 months ago by Marc Mezzarobba

Description: modified (diff)

comment:2 Changed 18 months ago by Samuel Lelièvre

Possibly related:

  • #30518: Eigenvectors over QQbar are incorrectly conjugated

comment:3 Changed 18 months ago by Vincent Delecroix

Indeed, the following method on gaussian integers looks bad

    def _symbolic_(self, SR):
        r"""
        EXAMPLES::

            sage: SR(1 + 2*i)
            2*I + 1
        """
        from sage.symbolic.constants import I
        return self[1]*I + self[0]

comment:4 Changed 18 months ago by Marc Mezzarobba

Hmm, if I remember right, the GaussianInteger class was intended for ℚ[i] with its standard embedding, so I think there is something else.

comment:5 Changed 18 months ago by Marc Mezzarobba

Authors: Marc Mezzarobba
Branch: u/mmezzarobba/ticket/31151
Commit: fddaa2c0b55dffaa4d3b1c616d291da18c583d98
Status: newneeds_review

...but I see no reason not to extend it to support both embeddings.


New commits:

fddaa2c#31151 support both embeddings in NumberFieldElement_gaussian

comment:6 Changed 18 months ago by Vincent Delecroix

Reviewers: Vincent Delecroix

looks good... waiting for patchbot.

comment:7 Changed 18 months ago by Vincent Delecroix

Status: needs_reviewpositive_review

comment:8 Changed 18 months ago by Marc Mezzarobba

thanks for the quick review!

comment:9 Changed 18 months ago by Marc Mezzarobba

Priority: majorcritical

Raising the priority to critical to stress that the fix really should go in Sage 9.3.

comment:10 Changed 18 months ago by Matthias Köppe

Priority: criticalblocker

Setting priority to blocker to bring this ticket to the attention of the release bot.

comment:11 Changed 18 months ago by Volker Braun

Branch: u/mmezzarobba/ticket/31151fddaa2c0b55dffaa4d3b1c616d291da18c583d98
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.