Opened 13 years ago
Closed 13 years ago
#3676 closed enhancement (fixed)
[with patch, positive review] Refactor graph isom code.
| Reported by: | rlm | Owned by: | rlm |
|---|---|---|---|
| Priority: | major | Milestone: | sage-3.1 |
| Component: | combinatorics | Keywords: | graphs |
| Cc: | boothby | Merged in: | |
| Authors: | Reviewers: | ||
| Report Upstream: | Work issues: | ||
| Branch: | Commit: | ||
| Dependencies: | Stopgaps: |
Status badges
Description
After this patch, graph_isom will be essentially obsolete. Brought to the GNU General Public by Google, Inc.
Attachments (7)
Change History (21)
Changed 13 years ago by
comment:1 Changed 13 years ago by
- Cc boothby added
comment:2 Changed 13 years ago by
comment:3 Changed 13 years ago by
David,
the above doctest is a known issue and orthogonal to rlm's code. See #3634 for the patch that likely caused this.
Cheers,
Michael
comment:4 Changed 13 years ago by
Okay. I was just trying to help out with the doctesting, that's all. Seems like it tests fine then.
Changed 13 years ago by
comment:5 Changed 13 years ago by
It's so... readable...
Changed 13 years ago by
comment:6 Changed 13 years ago by
The patches here may depend on #3703.
Changed 13 years ago by
comment:7 Changed 13 years ago by
- Milestone changed from sage-3.1.1 to sage-3.1
Changed 13 years ago by
comment:8 Changed 13 years ago by
I can flatten those last three if desired...
comment:9 Changed 13 years ago by
Changed 13 years ago by
comment:10 Changed 13 years ago by
- Summary changed from [with patch, needs review] Refactor graph isom code. to [with patch, needs review, review in progress] Refactor graph isom code.
3676-ncalexan-docstring-changes.patch changes some documentation to be clearer.
I am happy with this patch, save for a missing module docstring. rlmiller will write said docstring, explaining programming API to his code, and then this is ready for showtime.
Apply all patches in order.
comment:11 Changed 13 years ago by
Looks good to me.
comment:12 Changed 13 years ago by
The last patch is a flattened version of the previous ones, together with a recipe for implementing other objects. It should be finally ready to go. Apply only the last patch.
comment:13 Changed 13 years ago by
- Summary changed from [with patch, needs review, review in progress] Refactor graph isom code. to [with patch, positive review] Refactor graph isom code.
rlm and I have gone back and forth on this and I think it's great. I say apply!
comment:14 Changed 13 years ago by
- Resolution set to fixed
- Status changed from new to closed
Merged in Sage 3.1.alpha2
FYI, I got the following test failure:
wdj@hera:~/sagefiles/sage-3.0.4.rc0$ ./sage -t devel/sage/sage/rings/finite_field_ntl_gf2e.pyx sage -t devel/sage/sage/rings/finite_field_ntl_gf2e.pyx ********************************************************************** File "/home/wdj/sagefiles/sage-3.0.4.rc0/tmp/finite_field_ntl_gf2e.py", line 170: sage: k.modulus() Expected: x^17 + x^16 + x^15 + x^10 + x^8 + x^6 + x^4 + x^3 + x^2 + x + 1 Got: x^17 ********************************************************************** 1 items had failures: 1 of 7 in __main__.example_2 ***Test Failed*** 1 failures. For whitespace errors, see the file /home/wdj/sagefiles/sage-3.0.4.rc0/tmp/.doctest_finite_field_ntl_gf2e.py [2.9 s] exit code: 1024 ---------------------------------------------------------------------- The following tests failed: sage -t devel/sage/sage/rings/finite_field_ntl_gf2e.pyx Total time for all tests: 2.9 seconds