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

Simplify NumberFieldIdeal.gens_reduced() #9753

Closed
jdemeyer opened this issue Aug 15, 2010 · 12 comments
Closed

Simplify NumberFieldIdeal.gens_reduced() #9753

jdemeyer opened this issue Aug 15, 2010 · 12 comments
Assignees
@loefflerd
Labels
c: number fields p: major / 3 t: enhancement
Milestone
sage-4.6

Comments

@jdemeyer
Copy link

The function NumberFieldIdeal.gens_reduced() can be simplified quite a bit without essentially changing its functionality. We can also add a new function gens_two() which writes a number field ideal using two generators, like PARI's idealtwoelt().

Dependencies: #9400, #9898

Component: number fields

Keywords: number field ideal gens_two idealtwoelt

Author: Jeroen Demeyer

Reviewer: David Loeffler

Merged: sage-4.6.alpha2

Issue created by migration from https://trac.sagemath.org/ticket/9753
@jdemeyer jdemeyer added this to the sage-4.6 milestone Aug 15, 2010
@jdemeyer
Copy link
Author

Author: Jeroen Demeyer

@jdemeyer
Copy link
Author

comment:1

I will wait to fix the doctests until after #9343 and #9400.

@jdemeyer

This comment has been minimized.

Sign in to view
@jdemeyer
Copy link
Author

Attachment: 9753.patch.gz

Adds function gens_two(), rewrites gens_reduced() and fixes doctests

@jdemeyer

This comment has been minimized.

Sign in to view
@jdemeyer
Copy link
Author

Changed keywords from none to number field ideal gens_two idealtwoelt

@loefflerd
Copy link
Mannequin

loefflerd mannequin commented Sep 23, 2010

Reviewer: David Loeffler

@loefflerd
Copy link
Mannequin

loefflerd mannequin commented Sep 23, 2010

comment:6

Looks fine to me, and all tests pass on my machine.

@qed777
Copy link
Mannequin

qed777 mannequin commented Sep 28, 2010

comment:7

Could someone update the patch commit string with a more descriptive first line (still including the ticket number) and restore the positive review?

@qed777 qed777 mannequin added s: needs work and removed s: positive review labels Sep 28, 2010
@loefflerd
Copy link
Mannequin

loefflerd mannequin commented Sep 28, 2010

Attachment: 9753-better_commit_string.patch.gz

New version with better commit string

@loefflerd
Copy link
Mannequin

loefflerd mannequin commented Sep 28, 2010

comment:8

Done.

@qed777
Copy link
Mannequin

qed777 mannequin commented Sep 28, 2010

Merged: sage-4.6.alpha2

@qed777 qed777 mannequin removed the s: positive review label Sep 28, 2010
@qed777 qed777 mannequin closed this as completed Sep 28, 2010
@JohnCremona JohnCremona mentioned this issue Sep 28, 2010
Closed
@roed314 roed314 mentioned this issue Sep 8, 2014
Closed
@jdemeyer jdemeyer mentioned this issue Sep 27, 2010
Closed
@loefflerd loefflerd mannequin mentioned this issue Sep 23, 2010
Closed
This was referenced Nov 15, 2010
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@loefflerd loefflerd

Labels
c: number fields p: major / 3 t: enhancement
Projects
None yet
Milestone
sage-4.6
Development

No branches or pull requests
1 participant
@jdemeyer