Ticket #10699 (closed enhancement: fixed)

Opened 2 years ago

Last modified 2 years ago

Misc improvements to the code of sage/combinat/e_one_star.py

Reported by: slabbe Owned by: slabbe, tjolivet
Priority: major Milestone: sage-4.6.2
Component: combinatorics Keywords:
Cc: tjolivet Work issues:
Report Upstream: N/A Reviewers: Sébastien Labbé, Timo Jolivet
Authors: Timo Jolivet, Sébastien Labbé Merged in: sage-4.6.2.alpha4
Dependencies: Stopgaps:

Description (last modified by tjolivet) (diff)

The file e_one_star.py was merged recently into sage. After using it a bit for our research, we suggest the following improvements:

  • Add extra_code_before and extra_code_after arguments to the plot_tikz method.
  • Macros don't work when using sagetex. Remove the macros loza, lozb, lozc: just print the plain tikz code.
  • Allow the options iterations=0 in the __call__ method of E1Star.
  • Be able to plot patches made of unit segments (discrete lines).
  • Add the Patch method .occurences_in which returns the positions at which a patch appears in an other patch.
  • Change the default color map for the .repaint method (it's just nicer).
  • Add the .faces_of_vector method for Patch.
  • Fix a small bug: allow the type of a face and of the letters of a substitution to be int, and not only Integer.
  • Some other small fixes and documentation details.

Attachments

trac_10699_e_one_star_fix-sl.patch Download (4.1 KB) - added by slabbe 2 years ago.
First patch.
trac_10699_small_fixes-sl.patch Download (3.4 KB) - added by slabbe 2 years ago.
Applies over the precedent 2 patches
trac_10699_e_one_star_fix-tj.patch Download (35.1 KB) - added by tjolivet 2 years ago.
applies over the first patch

Change History

Changed 2 years ago by slabbe

First patch.

comment:1 Changed 2 years ago by slabbe

  • Cc tjolivet added

comment:2 Changed 2 years ago by tjolivet

  • Description modified (diff)

comment:3 Changed 2 years ago by slabbe

  • Status changed from new to needs_review

Changed 2 years ago by slabbe

Applies over the precedent 2 patches

comment:4 Changed 2 years ago by slabbe

I just added a patch which fixes some small issues. All test passed. Documentation builds fine. I am OK with giving this ticket a positive review if Timo agrees with my two patches.

comment:5 Changed 2 years ago by tjolivet

  • Status changed from needs_review to positive_review

Tests passed and I agree with the patches.

comment:6 follow-up: ↓ 7 Changed 2 years ago by jdemeyer

  • Status changed from positive_review to needs_work
  • Reviewers set to Sébastien Labbé, Timo Jolivet
  • Authors set to Timo Jolivet, Sébastien Labbé

Timo: please fix the commit message of your patch (use hg qrefresh -e for that).

Also, remember to fill in your names as Author/Reviewer?.

comment:7 in reply to: ↑ 6 Changed 2 years ago by tjolivet

  • Status changed from needs_work to positive_review

Replying to jdemeyer:

Timo: please fix the commit message of your patch (use hg qrefresh -e for that).

OK it's done.

Changed 2 years ago by tjolivet

applies over the first patch

comment:8 Changed 2 years ago by tjolivet

I just re-uploaded the patch trac_10699_e_one_star_fix-tj.patch to correct a minor typo in a function name (occurences_of instead of occurrences_of).

comment:9 Changed 2 years ago by jdemeyer

  • Status changed from positive_review to closed
  • Resolution set to fixed
  • Merged in set to sage-4.6.2.alpha4
Note: See TracTickets for help on using tickets.