Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

a quick way to create trees #15121

Closed
fchapoton opened this issue Aug 29, 2013 · 12 comments
Closed

a quick way to create trees #15121

fchapoton opened this issue Aug 29, 2013 · 12 comments

Comments

@fchapoton
Copy link
Contributor

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

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

CC: @sagetrac-sage-combinat

Component: combinatorics

Keywords: trees

Author: Frédéric Chapoton

Reviewer: Travis Scrimshaw

Merged: sage-5.13.beta2

Issue created by migration from https://trac.sagemath.org/ticket/15121

@fchapoton fchapoton added this to the sage-5.13 milestone Aug 29, 2013
@fchapoton
Copy link
Contributor Author

Dependencies: #11529

@fchapoton
Copy link
Contributor Author

Changed dependencies from #11529 to none

@fchapoton
Copy link
Contributor Author

comment:2

now ready for review !

But where is the bot ? Still at the beach ?

@fchapoton

This comment has been minimized.

@fchapoton fchapoton changed the title a quick way to create tree a quick way to create trees Sep 15, 2013
@fchapoton
Copy link
Contributor Author

Changed keywords from tree to trees

@tscrim
Copy link
Collaborator

tscrim commented Oct 23, 2013

comment:5

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

@fchapoton
Copy link
Contributor Author

Attachment: trac_15121_hexacode_for_trees-fc.patch.gz

@fchapoton
Copy link
Contributor Author

comment:6

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..

@tscrim
Copy link
Collaborator

tscrim commented Oct 23, 2013

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Oct 23, 2013

comment:7

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

Looks good to me. Thanks.

@fchapoton
Copy link
Contributor Author

comment:8

Thanks a lot Travis.

@jdemeyer
Copy link

Merged: sage-5.13.beta2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants