-
Notifications
You must be signed in to change notification settings - Fork 46
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
Add HaarGraph #408
Add HaarGraph #408
Conversation
I think documentation would be really helpful to reviewing this graph, since I'm not sure what you're using to construct the Haar graph. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have consulted Wolfram Alpha to see what this graph is about, and it seems to agree with the definition. However, if we proceed according to what was said in the meeting today, documentation should be added and also this function should be declared as a constructor and operations that return the final mutable and immutable version of the digraph should be added :)
Ok sounds good! I'll have a look at some examples and try to fix the documentation and function declaration... |
DIGRAPHS_BlistNumber([IsInt], [IsInt]) is taken from a GAP Semigroups function of the same name and is not meant to be user facing here, so I'm assuming it's meant to stay a global function. |
Closing and reopening to (hopefully) re-trigger the CI. Note, the Azure Pipelines jobs were not starting...
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is looking nice! Thanks. I'll commit these changes, and then squash, rebase and onto master, and push... and then merge if all seems well 🙂
@finnbuck I'll note that the documentation you wrote wasn't actually linked into the manual - see for reference: https://github.com/digraphs/Digraphs/pull/425/files#r592297697 But I'll do that for you now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hopefully it's good to go!
Add global function for generating a Haar Graph based on a single parameter. @flsmith