Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#12380 closed enhancement (fixed)

Move methods from Word_nfactor_enumerable to FiniteWord_class

Reported by: slabbe Owned by: slabbe
Priority: major Milestone: sage-5.3
Component: combinatorics Keywords:
Cc: abmasse, vdelecroix Merged in: sage-5.3.beta2
Authors: Sébastien Labbé Reviewers: André Apitzsch
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #9958, #13073 Stopgaps:

Description (last modified by jdemeyer)

The file sage/combinat/words/nfactor_enumerable_word.py and its class Word_nfactor_enumerable were introduced by ticket #8604. Just when it got merged into Sage, I realized it was a mistake. This class should not exist. The fact that its name was ugly was an indice. All those methods should be in a Language class (see #12225). It would get things a lot cleaner.

Only the class FiniteWord_class was inheriting from it, so it farly easy to fix : just move the 14 methods from one class to the other.

Note : this change will be unnoticed by the user.

Apply: 12380_move_nfactors_methods-sl-updated.patch and trac_12380_reviewer.patch

Attachments (2)

12380_move_nfactors_methods-sl-updated.patch (48.2 KB) - added by slabbe 8 years ago.
trac_12380_reviewer.patch (6.4 KB) - added by jdemeyer 8 years ago.
Remove trailing white spaces

Download all attachments as: .zip

Change History (12)

comment:1 Changed 8 years ago by slabbe

  • Dependencies set to #9958

Depends on one of the patch of #9958 merged in sage-5.0.beta0, namely : 9958_combinat.patch.

comment:2 Changed 8 years ago by slabbe

  • Authors set to Sébastien Labbé
  • Cc ablondin vdelecroix added
  • Description modified (diff)
  • Status changed from new to needs_review

comment:3 Changed 8 years ago by aapitzsch

  • Reviewers set to André Apitzsch

Your patch looks good. If you are okay with my one, you can change the status to positive review.

comment:4 Changed 8 years ago by slabbe

  • Dependencies changed from #9958 to #9958, #13073

Ticket #13073 just changed one line in the file sage/combinat/words/nfactor_enumerable_word.py, so the patch here does not apply anymore. I just updated the patch (under another name) with the following affected changes :

$ diff 12380_move_nfactors_methods-sl.patch 12380_move_nfactors_methods-sl-updated.patch 
426c426
< +        from sage.graphs.all import DiGraph
---
> +        from sage.graphs.digraph import DiGraph
1181c1181
< -        from sage.graphs.all import DiGraph
---
> -        from sage.graphs.digraph import DiGraph

The reviewer's patch still applies over the new updated one. I agree with the changes made by the reviewer. All tests pass in sage/combinat/words. Documentation builds fine. I give a positive review to the reviewer's patch.

Apply 12380_move_nfactors_methods-sl-updated.patch trac_12380_reviewer.patch

Because of changes made for #13073, I will not change the status of the ticket to positive review. I give André a last look at it.

Last edited 8 years ago by slabbe (previous) (diff)

comment:5 Changed 8 years ago by slabbe

  • Description modified (diff)

comment:6 Changed 8 years ago by slabbe

  • Cc abmasse added; ablondin removed

comment:7 Changed 8 years ago by aapitzsch

  • Status changed from needs_review to positive_review

comment:8 Changed 8 years ago by jdemeyer

  • Description modified (diff)

comment:9 Changed 8 years ago by jdemeyer

  • Merged in set to sage-5.3.beta2
  • Resolution set to fixed
  • Status changed from positive_review to closed

Changed 8 years ago by jdemeyer

Remove trailing white spaces

comment:10 Changed 8 years ago by jdemeyer

Rebased reviewer patch to sage-5.3.beta1.

Note: See TracTickets for help on using tickets.