-
Notifications
You must be signed in to change notification settings - Fork 31
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
Path Complex #238
Path Complex #238
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #238 +/- ##
==========================================
+ Coverage 87.89% 89.59% +1.70%
==========================================
Files 26 28 +2
Lines 2858 3326 +468
==========================================
+ Hits 2512 2980 +468
Misses 346 346
☔ View full report in Codecov by Sentry. |
Please add the following methods : (1) to_other_complex() where other_complex is any more general of path complex. e.g colored HG and combinatorial complex. (2) restrict_to_nodes (3) restrict to paths (4) set attrs and get attrs (similar to TopoNetX/toponetx/classes/cell_complex.py Line 957 in da155df
(5) add hodge_laplacian_spectrum methods in the file : https://github.com/pyt-team/TopoNetX/blob/main/toponetx/algorithms/spectrum.py (6) add tutorial on path complex demonstrating the main methods. |
@quang-truong Finally for adj/incidence/laplacian matrices please add weight parameter to make the signature consistent across the package #242. Also please do not forget to cite the paper https://arxiv.org/pdf/1207.2834.pdf along your paper in the documentation for PathComplex. |
Regarding (1), the current implementation of Besides that, all of the above methods are implemented. I am working on (6), which will take a while. |
@ffl096 can you please take a look at this one when you get a chance, give your comments ? Thanks! |
@ninamiolane @michaelschaub any high level comments? |
Co-authored-by: Florian Frantzen <2105496+ffl096@users.noreply.github.com>
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.
Some things I didn't notice before, sorry! Other than that, this pull request looks good to me :)
@quang-truong we are aiming to publish the package soon and the publisher requires almost 100% code coverage, can you try to increase the code coverage of path complex to its maximal ? thank you |
@mhajij I just add more test cases to cover all possible if statements and errors. |
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.
Excellent, thank you very much for this contribution!
This pull request contains source code to implement the Path Complex based on simple paths, which is inspired from the original Path Complex.