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.

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

problem parsing arguments to NumberField.order() #2480

Closed
ncalexan mannequin opened this issue Mar 12, 2008 · 8 comments
Closed

problem parsing arguments to NumberField.order() #2480

ncalexan mannequin opened this issue Mar 12, 2008 · 8 comments

Comments

@ncalexan
Copy link
Mannequin

ncalexan mannequin commented Mar 12, 2008

sage: y = ZZ['y'].0; K = NumberField(y^4 + 4*y^2 + 2, 'a'); K
Number Field in a with defining polynomial y^4 + 4*y^2 + 2
sage: B = K.integral_basis()
sage: B
[1, a, a^2, a^3]
sage: K.order(B)
Order in Number Field in a with defining polynomial y^4 + 4*y^2 + 2
sage: K.order(gens=B)
+Infinity

CC: @ncalexan @robertwb @mwhansen

Component: number fields

Keywords: number field order arguments

Author: Craig Citro

Reviewer: Mike Hansen

Merged: sage-4.3.2.alpha0

Issue created by migration from https://trac.sagemath.org/ticket/2480

@ncalexan ncalexan mannequin added this to the sage-4.3.2 milestone Mar 12, 2008
@ncalexan ncalexan mannequin assigned williamstein Mar 12, 2008
@loefflerd loefflerd mannequin assigned loefflerd and unassigned williamstein Jul 20, 2009
@craigcitro
Copy link
Member

comment:2

This wasn't so bad -- the problem was that gens= put the list of gens in the kwds dict, instead of in the *-argument. I've attached a fix, but I'd love for someone to tell me if deleting gens out of the kwds dict is sufficiently pythonic. (If you don't, the call to absolute_order_from_ring_generators rightfully complains that gens is specified twice.) Another option would be to reassign kwds['dict'] at the end, but I don't think that's any nicer. (In fact, that might be epsilon slower, since it's another argument to unpack from the dictionary on the other side.)

Also, the comment block in the docstring really looks like something was accidentally cut off at some point. Amusingly, this isn't the case: I actually dug through the hg logs, and it was really committed just like that.

@craigcitro
Copy link
Member

Author: Craig Citro

@craigcitro
Copy link
Member

comment:3

Mike and Robert, I'm adding you on the cc so that you can tell me if I'm being sufficiently pythonic. :)

@mwhansen
Copy link
Contributor

comment:4

Hey Craig,

gens = kwds.pop('gens')

is probably better.

@mwhansen
Copy link
Contributor

comment:5

Err,

gens = kwds.pop('gens', args)

@craigcitro
Copy link
Member

comment:6

Attachment: trac_2480.patch.gz

Nice. New patch with Mike's suggestion incorporated posted.

@mwhansen
Copy link
Contributor

Reviewer: Mike Hansen

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Jan 24, 2010

Merged: sage-4.3.2.alpha0

@sagetrac-mvngu sagetrac-mvngu mannequin closed this as completed Jan 24, 2010
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants