# Ticket #2480(closed defect: fixed)

Opened 5 years ago

## problem parsing arguments to NumberField.order()

Reported by: Owned by: ncalexan davidloeffler minor sage-4.3.2 number fields number field order arguments ncalexan, robertwb, mhansen N/A Mike Hansen Craig Citro sage-4.3.2.alpha0

### Description

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

## Change History

### comment:1 Changed 4 years ago by davidloeffler

• Owner changed from was to davidloeffler
• Component changed from number theory to number fields

### comment:2 Changed 3 years ago by craigcitro

• Status changed from new to needs_review
• Report Upstream set to N/A
• Authors set to Craig Citro

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.

### comment:3 Changed 3 years ago by craigcitro

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

### comment:4 Changed 3 years ago by mhansen

Hey Craig,

gens = kwds.pop('gens')

is probably better.

### comment:5 Changed 3 years ago by mhansen

Err,

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

### comment:6 Changed 3 years ago by craigcitro

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

### comment:7 Changed 3 years ago by mhansen

• Status changed from needs_review to positive_review
• Reviewers set to Mike Hansen

### comment:8 Changed 3 years ago by mvngu

• Status changed from positive_review to closed
• Resolution set to fixed
• Merged in set to sage-4.3.2.alpha0
Note: See TracTickets for help on using tickets.