fix: HG using SC as data

[thomasrobiglio]

Related to #398. The bug was coming from the condition I had set in the convert_to_hypergraph() in PR #345 (sry 😢). Now it should be fixed.

There are two issues related to this:

1. The constructors xgi.Hypergraph(SC) and xgi.SimplicialComplex(H) are not tested!
2. How it is implemented now if I call the cover convert_to_hypergraph() as a standalone with a SC as an object I get back the HG constructed from the maximal simplices of the SC, if instead, I call it via the constructor I get a HG in which every simplex in the SC becomes an edge. Is this ok or we want to change it?

For me it is intuitive to have the standalone convert give only the maximal simplices but I guess this is debatable.

[codecov]

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.01 ⚠️

Comparison is base (d7e59cc) 90.95% compared to head (186b1b7) 90.95%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #399      +/-   ##
==========================================
- Coverage   90.95%   90.95%   -0.01%     
==========================================
  Files          47       47              
  Lines        3882     3893      +11     
==========================================
+ Hits         3531     3541      +10     
- Misses        351      352       +1     

Impacted Files	Coverage Δ
xgi/convert.py	94.71% <100.00%> (-0.17%)	⬇️

... and 2 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.

[nwlandry]

Yeah, they should at least be consistent. I think the most natural (although the most dense) would be to keep all the subfaces when converting to a hypergraph. Perhaps you could add optional argument (maybe keep_subfaces) specifying whether to keep the subfaces or not with the default value being True

[thomasrobiglio]

Ok, I think a good solution would be:

• no ambiguity in convert_to_hypergraph between constructor and standalone call
• make from_max_simplices more discoverable.

Let me know what you think about it.

[nwlandry]

Hi @thomasrobiglio - I like your solution! Once Max is okay with this, it's ready to squash and merge.

[maximelucas]

Nice work thanks Thomas! I agree:

• convert_to_hypergraph(S) and Hypergraph(S) should do the same.
• if a function keeps only the maximal simplices, it should be explicit. As you found out Thomas, we have from_max_simplices that does that. Good idea making it more discoverable.
• great to add tests for the constructors so we won't break them in the future!

Your solution looks good. Just a few comments to make the tests more precise, otherwise good to go.