Opened 10 years ago
Closed 9 years ago
#7883 closed enhancement (fixed)
Added some functionality to ideals
Reported by: | robertwb | Owned by: | malb |
---|---|---|---|
Priority: | major | Milestone: | sage-4.6 |
Component: | commutative algebra | Keywords: | ideals |
Cc: | roed | Merged in: | sage-4.6.alpha2 |
Authors: | David Roe | Reviewers: | Robert Bradshaw, David Loeffler |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
Added some functionality to ideals (is_maximal works more often now, and there's a new ideal class for univariate polynomial rings). Changed the logic on what class is chosen for ideals so that it's easier to override with _ideal_class_. Number of generators?
Attachments (4)
Change History (12)
Changed 10 years ago by
comment:1 Changed 10 years ago by
- Description modified (diff)
- Status changed from new to needs_work
comment:2 Changed 10 years ago by
I haven't made the changes Robert suggested, but I wanted to get an updated patch up since other finite field functionality depends on this.
comment:3 Changed 10 years ago by
Part of a series:
8218 -> 8332 -> 7880 -> 7883 -> 8333 -> 8334 -> 8335
I tried to make each of these mostly self contained, with doctests passing after every ticket, but I didn't entirely succeed. If you're reviewing one of these tickets, applying later tickets will hopefully fix doctest failures that you're seeing.
comment:4 Changed 10 years ago by
Tried this under 4.3.4.rc0 with 8218, 8332, 7880 applied. It applies fine but seven doctests fail. Moreover, I second Robert's criticism: it's not clear what all this new code is actually for. (The fact that other finite-field stuff depends on this doesn't change that.) Can we have a new patch with some more docstrings etc?
comment:5 Changed 9 years ago by
- Status changed from needs_work to needs_review
comment:6 Changed 9 years ago by
- Keywords ideals added
- Reviewers changed from Robert Bradshaw to Robert Bradshaw, David Loeffler
- Status changed from needs_review to positive_review
I've uploaded a patch above which consists of
- the two patches mentioned in roed's previous comment
- the hunk from
7585_12_1_fixes.2.patch
(at #8334) that concernssage/rings/ring.pyx
- a tiny fix to remove the code in
ideal_1poly_field
that callsresidue_field
, since the residue fields code isn't going in until a subsequent patch.
I have checked that this applies cleanly to 4.6.alpha1 and passes long doctests.
comment:7 Changed 9 years ago by
- Description modified (diff)
comment:8 Changed 9 years ago by
- Merged in set to sage-4.6.alpha2
- Resolution set to fixed
- Status changed from positive_review to closed
There's a lot of useful looking code in here, but it's unclear what exactly it does or it's intended to fix. Some more explanations would be nice, and the new behavior should be doctested. Especially for most of the changes in
sage/rings/ideal.py
What is the parameter in _ideal_class_ supposed to mean? (Then why is it 0 by default?)
Depends on #7881?