Opened 11 years ago

Closed 11 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:

Description

After this patch, graph_isom will be essentially obsolete. Brought to the GNU General Public by Google, Inc.

Attachments (7)

trac3676-refactor_graph_isom.patch (79.4 KB) - added by rlm 11 years ago.
trac3676-bitset_pxd.patch (3.8 KB) - added by rlm 11 years ago.
trac3676-cleanup.patch (3.4 KB) - added by rlm 11 years ago.
trac3676-cleanup2.patch (3.9 KB) - added by rlm 11 years ago.
trac3676-cleanup3.patch (1.2 KB) - added by rlm 11 years ago.
3676-ncalexan-docstring-changes.patch (4.8 KB) - added by ncalexan 11 years ago.
trac_3676-graph_isom_refactor.patch (88.0 KB) - added by rlm 11 years ago.
Apply only this patch (please do not delete the others!)

Download all attachments as: .zip

Change History (21)

Changed 11 years ago by rlm

comment:1 Changed 11 years ago by rlm

  • Cc boothby added

comment:2 Changed 11 years ago by wdj

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

comment:3 Changed 11 years ago by mabshoff

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 11 years ago by wdj

Okay. I was just trying to help out with the doctesting, that's all. Seems like it tests fine then.

Changed 11 years ago by rlm

comment:5 Changed 11 years ago by boothby

It's so... readable...

Changed 11 years ago by rlm

comment:6 Changed 11 years ago by rlm

The patches here may depend on #3703.

Changed 11 years ago by rlm

comment:7 Changed 11 years ago by rlm

  • Milestone changed from sage-3.1.1 to sage-3.1

Changed 11 years ago by rlm

comment:8 Changed 11 years ago by rlm

I can flatten those last three if desired...

comment:9 Changed 11 years ago by mabshoff

Once this ticket is merged #3786 should be next.

Cheers,

Michael

Changed 11 years ago by ncalexan

comment:10 Changed 11 years ago by ncalexan

  • 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 11 years ago by boothby

Looks good to me.

comment:12 Changed 11 years ago by rlm

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.

Changed 11 years ago by rlm

Apply only this patch (please do not delete the others!)

comment:13 Changed 11 years ago by ncalexan

  • 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 11 years ago by mabshoff

  • Resolution set to fixed
  • Status changed from new to closed

Merged in Sage 3.1.alpha2

Note: See TracTickets for help on using tickets.