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

Template for removing examples #582

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

Jacob-Stevens-Haas
Copy link
Member

@Jacob-Stevens-Haas Jacob-Stevens-Haas commented Nov 7, 2024

As mentioned in #461, I'd like to remove a lot of the meat of examples/ because its growing too big and takes too much work to maintain along breaking changes. At the same time, I'd promised @znicolaou and others (including Nathan) that I would provide a way for people to keep their research reproducible and still a part of pysindy's docs, so long as they took care of any desired maintenance. #461 listed several criteria for the template repo, and I've finally built a solution that meets those criteria: https://github.com/dynamicslab/pysindy-example

Demonstration of yeeting example 3 into its own repo:
https://github.com/dynamicslab/sindy-original-example
Github pages lets you view that example as its own webpage (e.g. before PRing a new example to pysindy):
https://dynamicslab.github.io/sindy-original-example

  • external examples need to be registered in examples/external.yml
  • examples/README.rst needs to replace the explicit local link with our new custom sphinx directive, pysindy-example. BTW this now allows multiple notebooks for a single heading section, automatically.
  • If a submitter wants to maintain their example to keep up-to-date with pysindy breaking changes, there's instructions for how to connect CI. Each push to master on pysindy will trigger a CI run of all notebooks in dependent repositories.

@Jacob-Stevens-Haas Jacob-Stevens-Haas marked this pull request as ready for review November 7, 2024 04:14
@Jacob-Stevens-Haas Jacob-Stevens-Haas marked this pull request as draft November 7, 2024 04:14
@Jacob-Stevens-Haas
Copy link
Member Author

Still a draft - I need to add instructions to the README.md.

Copy link

codecov bot commented Nov 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.26%. Comparing base (23da590) to head (8ae0a0a).
Report is 22 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #582      +/-   ##
==========================================
- Coverage   95.30%   95.26%   -0.04%     
==========================================
  Files          37       39       +2     
  Lines        4046     4099      +53     
==========================================
+ Hits         3856     3905      +49     
- Misses        190      194       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Jacob-Stevens-Haas Jacob-Stevens-Haas marked this pull request as ready for review November 7, 2024 19:48
@znicolaou
Copy link
Collaborator

Thanks for organizing this, @Jacob-Stevens-Haas! I will do my job on it, but it might take a little while. Feel free to poke me if I'm holding up your work.

@Jacob-Stevens-Haas
Copy link
Member Author

Jacob-Stevens-Haas commented Nov 7, 2024

Not a problem Zach! 90% of the review is just in examples/README.rst and examples/external.yml to see what adding an external example looks like. The rest of the details are nuts and bolts of custom Sphinx directives and CI plumbing, which are only significant in what it means for workflow.

I wish I knew how to save the documentation build artifact so you could know how it looks, but here's some screenshots:

First item is internal example, second item is external example:


image

Current (readthedocs) version of that snip:


image

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.

2 participants