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

Updated docs with community section #72

Merged
merged 17 commits into from
Oct 30, 2023
Merged

Updated docs with community section #72

merged 17 commits into from
Oct 30, 2023

Conversation

niksirbi
Copy link
Member

The Community section

I updated the movement docs to add a new "Community" section.
It could also be called "About", but I prefer the "Community" term (borrowed from napari docs).

This section includes:

  • A community landing page, with links to the parts below and contact info
  • the statement on mission, scope, and design principles.
  • the roadmap
  • the contributing guide (it was a standalone page before, now I have moved it within this section)
  • Related projects
  • License (included from the file in the repository)

Many thanks to @adamltyson, @lochhh and @sfmig for helping me draft the mission, scope, design principles and roadmap!

Accompanying changes

The docs landing page and the repo README have also been updated to reflect the new structure of the docs. There is an "Overview" section in both, which contains the text @adamltyson wrote for the neuroinformatics main website. There is some regrettable repetition between these two documents (violating DRY), but including content from one of them to the other creates some nasty formatting issues (due to syntax differences between GitHub and Sphinx-Myst markdown). I think some repetition is fine for now, unless we want to write a GitHub action for building the README file automatically. Some links in the README will not work because they lead to the new website pages (which have not been deployed yet), but this should be fixed after merging and releasing.

I also was tempted to include some small docs fixes, e.g. changing the copyright holder to UCL, and making the URL for some pages more sensible.

What is missing from the Community section

How to review this PR

The best way is to build the docs website locally, following these instructions.

@codecov
Copy link

codecov bot commented Oct 25, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (1d4aba5) 98.05% compared to head (5b24fc7) 98.05%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #72   +/-   ##
=======================================
  Coverage   98.05%   98.05%           
=======================================
  Files           8        8           
  Lines         308      308           
=======================================
  Hits          302      302           
  Misses          6        6           

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

@niksirbi niksirbi marked this pull request as ready for review October 25, 2023 09:48
@niksirbi niksirbi requested review from lochhh and adamltyson October 25, 2023 09:48
Copy link
Member

@adamltyson adamltyson left a comment

Choose a reason for hiding this comment

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

LGTM! 🎉

docs/source/community/mission-scope.md Outdated Show resolved Hide resolved
@niksirbi niksirbi linked an issue Oct 25, 2023 that may be closed by this pull request
Copy link
Collaborator

@lochhh lochhh left a comment

Choose a reason for hiding this comment

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

Great work! 🚀

README.md Outdated Show resolved Hide resolved
docs/source/community/mission-scope.md Outdated Show resolved Hide resolved
docs/source/community/mission-scope.md Outdated Show resolved Hide resolved
docs/source/community/mission-scope.md Outdated Show resolved Hide resolved
docs/source/community/roadmap.md Outdated Show resolved Hide resolved
docs/source/community/roadmap.md Outdated Show resolved Hide resolved
docs/source/conf.py Show resolved Hide resolved
docs/source/index.md Outdated Show resolved Hide resolved
docs/source/snippets/status-warning.md Outdated Show resolved Hide resolved
docs/source/conf.py Outdated Show resolved Hide resolved
@niksirbi
Copy link
Member Author

niksirbi commented Oct 27, 2023

This error in building the docs took me forever to track down, but I now know its source.
Strap in, because it's a loong story:

Because our documentation includes examples built with Sphinx-Gallery, movement itself has to be included in docs/requirements.txt. This is achieved by adding -e . at the top of that file. So far this worked fine.

However, movement depends on sleap-io, which in turn depends on PyAV - a package providing Python bindings for ffmpeg. PyAV, which is available on pip as av, doesn't (yet) provide wheels for Python3.12.

Normally, this wouldn't be a problem for us, since we officially support Python versions 3.9 - 3.11. That said, the reusable workflow we use for building the Sphinx docs includes:

  - name: Setup Python
    uses: actions/setup-python@v4
    with:
      python-version: '3.x'

3.x means the latest stable version, and must have recently been changed to 3.12. So when this action runs, it tries to install movement -> sleap-io -> av in a Python 3.12 environment, and fails to find the wheels. I reproduced this locally on Ubuntu. The documentation build would fail on 3.12 and succeed on all previous versions.

We could solve this in a number of ways:

  • Modify the reusable Sphinx build action to use Python 3.11 (instead of the latest). This is probably the easiest option. Or even better, expose a Python version option for this workflow, so it can be flexibly chosen depending on the package.
  • Ask sleap-io developers to switch their dependency from PyAV to pyav - which is a recent fork and provides the right wheels. But this may be unnecessary and rushed, if we assume that PyAV will eventually add the wheels.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@niksirbi
Copy link
Member Author

I solved the issue by updating the GitHub action and running it with Python 3.11. Thanks @alessandrofelder for approving the action update. Merging now.

@niksirbi niksirbi merged commit 45955d3 into main Oct 30, 2023
24 checks passed
@niksirbi niksirbi deleted the scope-and-roadmap branch October 31, 2023 10:07
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.

Better define the package's scope
3 participants