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

Attempt at fixing convert #345

Merged
merged 11 commits into from
May 3, 2023
Merged

Attempt at fixing convert #345

merged 11 commits into from
May 3, 2023

Conversation

thomasrobiglio
Copy link
Collaborator

Hi all,
this is my attempt at addressing issue #327 and some other troubles I've found using the functions in the convert.py file.

Now it should be possible to:

  1. Convert SC to Hyperg. and viceversa (using from_max_simplices and an analogous criterion when going from the other way around i.e. hyperdges -> maximal faces then i had all subfaces)
  2. Convert from list, dict, and pd.DataFrame to Hypergraph as was possible before but without the issue of not returning anything. In this case if a pass an already existing Hypergraph instance then I have the in-place modification of the instance, else the function returns a new hypergraph()
  3. Same as (2) for list, pd.Dataframe -> SimplicialComplex()

Hopefully this is what you had in mind when discussing #327 and this can be a useful contribution.

@codecov
Copy link

codecov bot commented Apr 29, 2023

Codecov Report

Patch coverage: 96.42% and project coverage change: +0.03 🎉

Comparison is base (1191b60) 90.39% compared to head (b34fc4f) 90.42%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #345      +/-   ##
==========================================
+ Coverage   90.39%   90.42%   +0.03%     
==========================================
  Files          41       41              
  Lines        2988     3008      +20     
==========================================
+ Hits         2701     2720      +19     
- Misses        287      288       +1     
Impacted Files Coverage Δ
xgi/convert.py 95.61% <96.42%> (-0.06%) ⬇️

... and 12 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@leotrs
Copy link
Collaborator

leotrs commented May 1, 2023

Hi @thomasrobiglio ! Thanks so much for your work on this. We'll review it soon and get back to you.

@maximelucas
Copy link
Collaborator

Also linked to #330

Copy link
Collaborator

@leotrs leotrs left a comment

Choose a reason for hiding this comment

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

Awesome! Just a few minor nitpicks that would make me very happy if you could respond to :)

xgi/convert.py Outdated Show resolved Hide resolved
xgi/convert.py Show resolved Hide resolved
xgi/convert.py Outdated Show resolved Hide resolved
xgi/convert.py Outdated Show resolved Hide resolved
xgi/convert.py Outdated Show resolved Hide resolved
xgi/convert.py Outdated Show resolved Hide resolved
xgi/convert.py Outdated Show resolved Hide resolved
@leotrs leotrs merged commit b0b2d29 into xgi-org:main May 3, 2023
18 checks passed
@leotrs
Copy link
Collaborator

leotrs commented May 3, 2023

Thank you @thomasrobiglio ! We hope to see more of your contributions in the future. If you're interested in keeping up with development, feel free to join us on Zulip!

@thomasrobiglio thomasrobiglio deleted the fix_convert branch May 19, 2023 22:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants