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

Provides some further functionalities for combinatorial maps #14255

Closed
stumpc5 opened this issue Mar 11, 2013 · 8 comments
Closed

Provides some further functionalities for combinatorial maps #14255

stumpc5 opened this issue Mar 11, 2013 · 8 comments

Comments

@stumpc5
Copy link
Contributor

stumpc5 commented Mar 11, 2013

This patch provides some further functionalities for combinatorial maps.

CC: @sagetrac-chrisjamesberg @VivianePons

Component: combinatorics

Keywords: combinatorial map

Author: Christian Stump

Reviewer: Travis Scrimshaw

Merged: sage-5.9.beta1

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

@stumpc5 stumpc5 added this to the sage-5.9 milestone Mar 11, 2013
@tscrim
Copy link
Collaborator

tscrim commented Mar 11, 2013

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Mar 11, 2013

comment:2

Hey Christian,

Looks good but there's two minor docstring things. First is I don't understand this line 210 in unbounded_map():

This can then be used to be applied to an element.

Second (and more nitpicky and far less important), the first line is suppose to be in the "affirmative" according to python doc standards:

Return the unbounded..."

Thanks,

Travis

@stumpc5
Copy link
Contributor Author

stumpc5 commented Mar 11, 2013

comment:3

Replying to @tscrim:

thanks for your review, Travis!

This can then be used to be applied to an element.

means that this is now an unbounded function and it takes one input parameter, namely self. How do you think I should write that?

Return the unbounded..."

alright, I will do that... (but first waiting for your answer to the above).

@tscrim
Copy link
Collaborator

tscrim commented Mar 11, 2013

comment:4

How does this sound:

You can use this method to return a function which takes as input an
element in the domain of the combinatorial map. See the example below.

? Also could you put another blank line between this and the "Return" line? Thank you.

@stumpc5
Copy link
Contributor Author

stumpc5 commented Mar 11, 2013

Attachment: trac_14255-combinatorial_maps-cs.patch.gz

@stumpc5
Copy link
Contributor Author

stumpc5 commented Mar 11, 2013

comment:5

Replying to @tscrim:

How does this sound:

You can use this method to return a function which takes as input an
element in the domain of the combinatorial map. See the example below.

? Also could you put another blank line between this and the "Return" line? Thank you.

Fixed, thanks!

@tscrim
Copy link
Collaborator

tscrim commented Mar 11, 2013

comment:6

Thank you Christian.

@jdemeyer
Copy link

Merged: sage-5.9.beta1

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