DiHypergraph class

[nwlandry]

This is a preliminary attempt at addressing #2. Currently the following are implemented:

• Number of nodes and edges
• adding/removing a single node or iterable of nodes
• adding/removing a single edge or an iterable of edges
• copy, clear
• convert_to_dihypergraph in the convert module
• empty_dihypergraph in the classic generators
• DiNodeView and DiEdgeView with members, memberships, filterby, and other standard class methods
• DiNodeStat, DiEdgeStat, MultiDiNodeStat, MultiDiEdgeStat are now implemented
• degree, in_degree, out_degree, size, order, tail_size, head_size are implemented

I added Tutorial 8 to demonstrate some of this basic functionality.

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[nwlandry]

This cleans up PR #297, which I will now close.

[codecov]

Codecov Report

Patch coverage: 92.37% and project coverage change: +0.21 🎉

Comparison is base (387647a) 90.90% compared to head (d5f5a5a) 91.11%.

❗ Current head d5f5a5a differs from pull request most recent head 5e2b6f6. Consider uploading reports for the commit 5e2b6f6 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #372      +/-   ##
==========================================
+ Coverage   90.90%   91.11%   +0.21%     
==========================================
  Files          42       46       +4     
  Lines        3177     3771     +594     
==========================================
+ Hits         2888     3436     +548     
- Misses        289      335      +46     

Impacted Files	Coverage Δ
xgi/classes/reportviews.py	93.58% <ø> (ø)
xgi/stats/dinodestats.py	83.33% <83.33%> (ø)
xgi/classes/direportviews.py	83.42% <83.42%> (ø)
xgi/stats/__init__.py	85.02% <89.47%> (+0.36%)	⬆️
xgi/convert.py	94.88% <90.00%> (-0.74%)	⬇️
xgi/classes/dihypergraph.py	98.31% <98.31%> (ø)
xgi/classes/__init__.py	100.00% <100.00%> (ø)
xgi/classes/hypergraph.py	85.37% <100.00%> (ø)
xgi/generators/classic.py	98.07% <100.00%> (+0.20%)	⬆️
xgi/stats/diedgestats.py	100.00% <100.00%> (ø)

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

[nwlandry]

I believe that I implemented everything that we discussed, so should be ready for review.

[maximelucas]

Okay this is a long PR so it's hard to check everything in detail. A few thoughts:

1. what makes me trust that the code works: (i) most of it is kind of duplicates from Hypergraph code, (ii) the notebook shows working features, and (iii) we have tests.
2. the fact that a lot of the code feels like duplicate from existing classes does not feel great. Would there be a way to eventually put this duplicate code in a unique place and us it for Hypergraph and then use it twice as in and out for DiHypergraph? Would it be worth it really?
3. codecov goes down a little, could we add a few more tests so that it does not?

Otherwise, as we said, I'd be okay to release this and flag this in the docs as an experimental feature, and try it in the real world.

[leotrs]

Lots of awesome work here. Agree with Max, there's definitely a lot of duplicated code with Hypergraph.

[leotrs]

Lots of awesome work here. Agree with Max, there's definitely a lot of duplicated code with Hypergraph.

[nwlandry]

Okay this is a long PR so it's hard to check everything in detail. A few thoughts:

1. what makes me trust that the code works: (i) most of it is kind of duplicates from Hypergraph code, (ii) the notebook shows working features, and (iii) we have tests.
2. the fact that a lot of the code feels like duplicate from existing classes does not feel great. Would there be a way to eventually put this duplicate code in a unique place and us it for Hypergraph and then use it twice as in and out for DiHypergraph? Would it be worth it really?
3. codecov goes down a little, could we add a few more tests so that it does not?

Otherwise, as we said, I'd be okay to release this and flag this in the docs as an experimental feature, and try it in the real world.

1./2. I agree. The eventual goal is to figure out some kind of inheritance. That being said, because we have spent so much time polishing the Hypergraph class, I wanted to keep the two classes separate until we test this in the wild.
3. I agree. I will take some time to write some tests.

[nwlandry]

Okay, I think that this should be good to go pending the questions that I was unsure of. Thank you both for taking the time to review such a large PR!

[leotrs]

Awesome Nich. We do need a plan for determining when this class will be "ready", since for now we are adding it as an experimental feature.

Do any of yall @nwlandry @maximelucas plan on making heavy use of this class? It needs to be battle tested.

[leotrs]

Awesome Nich. We do need a plan for determining when this class will be "ready", since for now we are adding it as an experimental feature.

Do any of yall @nwlandry @maximelucas plan on making heavy use of this class? It needs to be battle tested.

[maximelucas]

Awesome Nich. We do need a plan for determining when this class will be "ready", since for now we are adding it as an experimental feature.

Good idea. What are standard ways?

Do any of yall @nwlandry @maximelucas plan on making heavy use of this class? It needs to be battle tested.

At the moment I don't.

[nwlandry]

Thanks! I am planning on using it for a project on metabolic networks and I'm certainly hoping to make large use of this class.

Great question. I'm not sure I can think of a concrete milestone to say when it's ready, but roughly speaking, it would be great to have a lot of the same functionality as Hypergraph (algorithms, drawing, etc.) and a stable internal representation before it's considered ready.

[maximelucas]

Thanks for the new tests Nich!

[maximelucas]

Oh btw, before merging, is it written anywhere that it's an experimental feature?
I didn't see it but may have missed it.

[nwlandry]

Oh btw, before merging, is it written anywhere that it's an experimental feature? I didn't see it but may have missed it.

Great point. I will add this to the documentation of the DiHypergraph class.

[nwlandry]

Oh btw, before merging, is it written anywhere that it's an experimental feature? I didn't see it but may have missed it.

Okay, I added warnings to the DiHypergraph, DiEdgeView, and DiNodeView classes. I will merge once all the tests pass!