-
Notifications
You must be signed in to change notification settings - Fork 11
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
Support non-simplicial complexes #26
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #26 +/- ##
==========================================
- Coverage 94.47% 91.74% -2.73%
==========================================
Files 12 12
Lines 398 412 +14
==========================================
+ Hits 376 378 +2
- Misses 22 34 +12
Continue to review full report at Codecov.
|
@@ -59,6 +68,36 @@ def phat_diagrams(simplices, show_inf=False, verbose=True): | |||
|
|||
return dgms | |||
|
|||
def _assert_simplicial(simplices): |
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.
Please run black
code formatter over these changes.
|
||
def diagrams(self, simplices=None, show_inf=False, verbose=True, simplicial=True): | ||
""" | ||
Redefine the Custom version of diagrams so that simplicial defaults to True, rather |
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.
Could you help me understand why this one should default to true while the others default to false?
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.
My thought was that in all of the others, the simplicial complex is being generated in a way that we know it will be a simplicial complex. I thought it would be faster to pass False
because we don't need to check it. My thought was not that the other ones are nonsimplicial, it's just that we don't need to assert that they are. For this one, we are generating the complex in a way that may not be simplicial. I assumed that most users would want the default behavior to be a simplicial complex, and they could choose to make something more subtle by passing simplicial=False
No description provided.