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

update index.rst to have dihypergraph in quick references #392

Merged

Conversation

thomasrobiglio
Copy link
Collaborator

I noticed that the DH does not appear in the quick references, I think this might fix it (but I'm not familiar at all with how Sphinx works, etc. so pls check it).

@maximelucas
Copy link
Collaborator

I saw it too but not sure we want it there since it's still experimental?

@leotrs
Copy link
Collaborator

leotrs commented Jun 8, 2023

Good point Max

@thomasrobiglio
Copy link
Collaborator Author

Ah right, I did not think about that

@leotrs
Copy link
Collaborator

leotrs commented Jun 8, 2023

@thomasrobiglio you were right - you can check the documentation built from this PR (instead of from the main branch) by clicking "docs/readthedocs.orx:xgi" under the "all checks have passed" button here in this PR.

Copy link
Collaborator

@nwlandry nwlandry left a comment

Choose a reason for hiding this comment

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

I think this is a good idea. While this is still an experimental feature, perhaps you can just add (Experimental feature!) or something like that after the text, so that users can still easily find it if they want to use it.

@codecov
Copy link

codecov bot commented Jun 9, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (8d29604) 91.02% compared to head (81efa70) 91.02%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #392   +/-   ##
=======================================
  Coverage   91.02%   91.02%           
=======================================
  Files          47       47           
  Lines        3878     3878           
=======================================
  Hits         3530     3530           
  Misses        348      348           

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

@thomasrobiglio
Copy link
Collaborator Author

Now it will appear like this... but maybe this is not what you had in mind @nwlandry (since also there was already the warning in the class docs). Then of course we can change the text, this one was co-authored by copilot :)

Screenshot 2023-06-09 alle 10 22 44

@leotrs
Copy link
Collaborator

leotrs commented Jun 9, 2023

I like the warning admonition. I would delete the middle sentence and leave the first and last.

@nwlandry
Copy link
Collaborator

nwlandry commented Jun 9, 2023

Looks great! I agree with Leo on deleting the middle sentence and then I think you're good to merge, @thomasrobiglio. Thanks so much!

@thomasrobiglio thomasrobiglio merged commit d7e59cc into xgi-org:main Jun 9, 2023
19 checks passed
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

4 participants