Opened 4 years ago

Closed 4 years ago

#15121 closed enhancement (fixed)

a quick way to create trees

Reported by: chapoton Owned by:
Priority: minor Milestone: sage-5.13
Component: combinatorics Keywords: trees
Cc: sage-combinat Merged in: sage-5.13.beta2
Authors: Frédéric Chapoton Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by chapoton)

I propose a method to create (unlabelled) trees conveniently.

One enters a sequence of hexadecimal numbers, and this is converted into a rooted tree.

Attachments (1)

trac_15121_hexacode_for_trees-fc.patch​ (5.0 KB) - added by chapoton 4 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 4 years ago by chapoton

  • Dependencies set to #11529

comment:2 Changed 4 years ago by chapoton

  • Dependencies #11529 deleted
  • Status changed from new to needs_review

now ready for review !

But where is the bot ? Still at the beach ?

comment:3 Changed 4 years ago by chapoton

  • Description modified (diff)
  • Summary changed from a quick way to create tree to a quick way to create trees

comment:4 Changed 4 years ago by chapoton

  • Keywords trees added; tree removed

comment:5 Changed 4 years ago by tscrim

Hey Frederic,

Could you move from_hexcode to AbstractTree so the parent arg becomes self? IMO this way it is more discoverable and OOP.

Also (more of a pet-peeve of mine, so you can ignore this), I don't like else: statements following 

if condition:
    ...
    return foo

(or an sequence of elif's) since it is unnecessary indentation.

Thanks,
Travis

Changed 4 years ago by chapoton

comment:6 Changed 4 years ago by chapoton

Humm, this does not seem possible, because there is no class AbstractTrees? (with an s at the end)

Indeed, it does not seem reasonable to put that into the class AbstractTree?, because it would mean one has to build a tree before one can build another one..

comment:7 Changed 4 years ago by tscrim

  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

Ah, my bad. I thought there was a common parent class.

Looks good to me. Thanks.

comment:8 Changed 4 years ago by chapoton

Thanks a lot Travis.

comment:9 Changed 4 years ago by jdemeyer

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