Opened 6 years ago
Closed 6 years ago
#1107 closed enhancement (fixed)
[with patch, with positive review] add minkowski bound function for number fields
Reported by: | was | Owned by: | was |
---|---|---|---|
Priority: | minor | Milestone: | sage-2.8.15 |
Component: | number theory | Keywords: | |
Cc: | Merged in: | ||
Authors: | Reviewers: | ||
Report Upstream: | Work issues: | ||
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
Add computation of Minkowski bound to number fields (very simple).
Attachments (1)
Change History (11)
comment:1 Changed 6 years ago by mabshoff
- Milestone changed from sage-2.9 to sage-2.8.13
comment:2 Changed 6 years ago by robertwb
comment:3 Changed 6 years ago by mabshoff
The patch no longer applies cleanly:
mabshoff@sage:/tmp/Work-mabshoff/release-cycles/sage-2.8.13.alpha0/devel/sage$ hg import minkowski.patch applying minkowski.patch patching file sage/rings/rational_field.py Hunk #1 succeeded at 298 with fuzz 2 (offset 23 lines). Hunk #2 FAILED at 362 Hunk #3 FAILED at 370 Hunk #4 FAILED at 378 3 out of 4 hunks FAILED -- saving rejects to file sage/rings/rational_field.py.rej abort: patch failed to apply
Cheers,
Michaell
comment:4 Changed 6 years ago by cwitty
- Summary changed from [with patch] add minkowski bound function for number fields to [with broken patch] add minkowski bound function for number fields
comment:5 Changed 6 years ago by was
cwitty: williamstein, did you notice mabshoff's comment on your #1107 patch? Evidently it no longer applies. [9:13pm] williamstein: Thanks. [9:16pm] williamstein: actually it's fine -- the one hunk that doesn't get applied with 1107 doesn't apply because it is already applied in the current sage. [9:16pm] williamstein: So it's OK. Just ignore the one hunk that fails.
comment:6 Changed 6 years ago by was
[9:17pm] cwitty: The three hunks that don't get applied, you mean? (Judging from mabshoff's comment.) [9:20pm] williamstein: Yes, that's what I meant. Thanks.
comment:7 Changed 6 years ago by cwitty
- Summary changed from [with broken patch] add minkowski bound function for number fields to [with patch] add minkowski bound function for number fields
comment:8 Changed 6 years ago by was
OK, I rebased it so I get credit :-)
comment:9 Changed 6 years ago by robertwb
- Summary changed from [with patch] add minkowski bound function for number fields to [with patch, with positive review] add minkowski bound function for number fields
Looks good to me.
comment:10 Changed 6 years ago by mabshoff
- Resolution set to fixed
- Status changed from new to closed
Merged in 2.8.15.alpha0.
Note: See
TracTickets for help on using
tickets.
The patch is good.