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.

Sign up for GitHub

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

Jump to bottom

DigraphFromDigraph6String ability to return mutable digraphs #262

Merged
merged 11 commits into from
Oct 2, 2019
Merged

DigraphFromDigraph6String ability to return mutable digraphs #262

merged 11 commits into from
Oct 2, 2019

Conversation

MTWhyte
Copy link
Collaborator

@MTWhyte MTWhyte commented Sep 25, 2019

This pull request adds the functionality for DigraphFromDigraph6String to return mutable digraphs.

@james-d-mitchell
Copy link
Member

Looks good! What about the other functions in the same file??

@james-d-mitchell james-d-mitchell added this to the 1.0.0 milestone Sep 25, 2019
wilfwilson
wilfwilson reviewed Sep 25, 2019
View reviewed changes
gap/io.gi Show resolved Hide resolved
wilfwilson
wilfwilson reviewed Sep 25, 2019
View reviewed changes
Copy link
Collaborator

@wilfwilson wilfwilson left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah thanks @MTWhyte, would be create if you could do the same for the "friends" of DigraphFromDigraph6String (eg DigraphFromGraph6String) too. Also, please update the documentation for each of them (as in digraph.xml and examples.xml) to show that these things now take an optional first argument.

wilfwilson
wilfwilson reviewed Sep 25, 2019
View reviewed changes
gap/io.gi Outdated Show resolved Hide resolved
@MTWhyte
Copy link
Collaborator Author

MTWhyte commented Sep 25, 2019

Great, I'll change the others too!

@wilfwilson
Copy link
Collaborator

Great, thanks a lot. Really handy.

@wilfwilson
Copy link
Collaborator

@MTWhyte do you mean you'll do it in this PR, or in a separate PR?

@MTWhyte
Copy link
Collaborator Author

MTWhyte commented Sep 27, 2019

@wilfwilson which do you think would be better?

@wilfwilson
Copy link
Collaborator

I think you may as well do it all in this PR, but I don’t mind if you want to do it separately.

@MTWhyte
Copy link
Collaborator Author

MTWhyte commented Oct 2, 2019

I've now updated the 'friends' too, namely DigraphFromGraph6String, DigraphFromSparse6String, DigraphFromDiSparse6String and DigraphFromPlainTextString.

wilfwilson
wilfwilson approved these changes Oct 2, 2019
View reviewed changes
Copy link
Collaborator

@wilfwilson wilfwilson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I'll squash and merge iff the tests pass.

@wilfwilson wilfwilson merged commit 4e56813 into digraphs:master Oct 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@james-d-mitchell james-d-mitchell james-d-mitchell left review comments

@wilfwilson wilfwilson wilfwilson approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@MTWhyte @james-d-mitchell @wilfwilson