Opened 6 years ago

Closed 4 years ago

#11407 closed enhancement (fixed)

Add normalization to clonable lists

Reported by: hivert Owned by: hivert
Priority: major Milestone: sage-5.12
Component: combinatorics Keywords: clone normalization Cernay2012, days49
Cc: sage-combinat, tscrim Merged in: sage-5.12.beta2
Authors: Florent Hivert Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #11308 Stopgaps:

Description (last modified by hivert)

The patch adds a new support class in list_clone.pyx for lists with a normalization methods. The normalization method is called both after creation and clone/modification. This will be needed for rooted trees.

I also take the opportunity of cleaning up the bad usage of assert in the examples and replaced them by if ... : raise ValueError.

Apply

Attachments (3)

trac_11407-list_clone_improve-fh-rebased.patch (50.5 KB) - added by chapoton 4 years ago.
trac_11407-review-ts.patch (805 bytes) - added by tscrim 4 years ago.
trac_11407-list_clone_improve-fh.patch (67.3 KB) - added by hivert 4 years ago.

Download all attachments as: .zip

Change History (21)

comment:1 Changed 6 years ago by hivert

  • Description modified (diff)

comment:2 Changed 6 years ago by hivert

  • Dependencies set to #11308
  • Description modified (diff)

comment:3 Changed 6 years ago by hivert

  • Description modified (diff)
  • Status changed from new to needs_review

comment:4 Changed 6 years ago by hivert

  • Keywords Cernay2012 added

comment:5 Changed 5 years ago by hivert

I just pushed a new patch which fixes the failing doctests

comment:6 Changed 5 years ago by davidloeffler

  • Status changed from needs_review to needs_work
  • Work issues set to rebase to current beta

Patch does not apply to 5.0.beta11, I'm afraid

comment:7 Changed 5 years ago by nborie

Message for Florent:

For information, your patch does not commute with mine about enumeration modulo the action of a permgroup. I try to push back my patch in the queue but i didn't manage to do it without breaking the queue... Sorry for that...

comment:8 Changed 4 years ago by chapoton

Hello Florent,

I rebased the patch on 5.8 but then sage fails to compile with the following messages.

sage/structure/list_clone.pyx:1826:10: Overriding final methods is not allowed

sage/structure/list_clone.pyx:1826:10: Only final types can have final Python (def/cpdef) methods

It seems that the problem is with the __exit__ method.

Changed 4 years ago by chapoton

comment:9 Changed 4 years ago by chapoton

  • Description modified (diff)

comment:10 Changed 4 years ago by hivert

  • Cc tscrim added
  • Description modified (diff)
  • Keywords days49 added
  • Status changed from needs_work to needs_review
  • Work issues rebase to current beta deleted

comment:11 Changed 4 years ago by chapoton

for the bot

apply trac_11407-list_clone_improve-fh.patch

Changed 4 years ago by tscrim

comment:12 Changed 4 years ago by tscrim

  • Description modified (diff)
  • Reviewers set to Travis Scrimshaw

Hey Florent,

Looks good minus the one missed change in the review patch I've uploaded. After you give it a quick check, and you set this to positive review.

Best,
Travis

For patchbot:

Apply: trac_11407-list_clone_improve-fh.patch trac_11407-review-ts.patch

Changed 4 years ago by hivert

comment:13 Changed 4 years ago by hivert

  • Description modified (diff)

comment:14 Changed 4 years ago by hivert

Hi travis,

I folded your patch and fixed the multiline doctests. I'm ok with you changes so please doublecheck and se positive review if everything is ok.

Florent

comment:15 Changed 4 years ago by tscrim

  • Status changed from needs_review to positive_review

Looks good to me. Thanks Florent.

For patchbot:

Apply: trac_11407-list_clone_improve-fh.patch

comment:16 Changed 4 years ago by hivert

Thanks for the review.

Cheers,

Florent

comment:17 Changed 4 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:18 Changed 4 years ago by jdemeyer

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