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

DOC: Solve Dependencies Conflicts and pyproject build #613

Merged
merged 11 commits into from
May 29, 2024

Conversation

phmbressan
Copy link
Collaborator

@phmbressan phmbressan commented May 29, 2024

Pull request type

  • Code changes (bugfix, features)
  • ReadMe, Docs and GitHub updates

Checklist

  • Docs have been reviewed and added / updated
  • Lint (black rocketpy/ tests/) has passed locally
  • All tests (pytest tests -m slow --runslow) have passed locally
  • CHANGELOG.md has been updated (if relevant)

Note: --runslow is currently failing.

Current behavior

An error occurs while trying building docs due to many reasons, including:

  • New release of docutils not supported by m2r2;
  • pyproject.toml requires conf.py changes;
  • lxml separated its html_clean to another repo for security.

All those library changes happened around late april and beginning of may.

New behavior

The following changes were made:

  • Limit docutils version to 0.20.1;
  • Add to docs/requirements the new lxml_html_clean;
  • Add PYTHONPATH to conf.py.

Following sphinx docs recommendation, in order to avoid breaking docs due to dependency updates pip-tools was used to pin down all docs dependencies.

The following procedure should be followed to add/update a docs dependency:

  1. Add the new top-level dependency to docs/requirements.in;
  2. Run pip-compile docs/requirements.in.
Docs are building now: ![image](https://github.com/RocketPy-Team/RocketPy/assets/87212571/3edb37d1-ec8a-48ab-b7ee-e13fa75e0137)

Breaking change

  • Yes
  • No

Additional information

The PR #584 removes README.md from the docs, allowing the removal of m2r2 dependency and the limitation of docutils version.

@phmbressan phmbressan added Bug Something isn't working Docs Docs and examples related labels May 29, 2024
@phmbressan phmbressan added this to the Release v1.X.0 milestone May 29, 2024
@phmbressan phmbressan self-assigned this May 29, 2024
@phmbressan phmbressan requested a review from a team as a code owner May 29, 2024 00:53
Copy link

codecov bot commented May 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.56%. Comparing base (836cde7) to head (73ec844).
Report is 1 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #613   +/-   ##
========================================
  Coverage    73.56%   73.56%           
========================================
  Files           70       70           
  Lines        10313    10313           
========================================
  Hits          7587     7587           
  Misses        2726     2726           

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

Copy link
Member

@Gui-FernandesBR Gui-FernandesBR left a comment

Choose a reason for hiding this comment

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

Really nice PR, thanks for the solution.

Would you mind adding a comment to ~ somewhere I don't know ~ saying that you need to run pip-compile ... to generate the requirements.txt file now?

Maybe at the top of the requirements.in ?

All tests are passing in the CI. I'm approving the PR, can be merged already I think.

.readthedocs.yaml Show resolved Hide resolved
docs/requirements.in Show resolved Hide resolved
@phmbressan
Copy link
Collaborator Author

There are some comments added automatically at the top of docs/requirements.txt which specify what you mentioned.

I don't know if you have seen those, but if you think that it is not enough, I can add a line on top of requirements.in with the same.

@Gui-FernandesBR
Copy link
Member

There are some comments added automatically at the top of docs/requirements.txt which specify what you mentioned.

I don't know if you have seen those, but if you think that it is not enough, I can add a line on top of requirements.in with the same.

OK, Good, that's enough, I haven't seen those before.
All good.

@phmbressan phmbressan merged commit 71810ab into develop May 29, 2024
10 checks passed
@phmbressan phmbressan deleted the bug/docs-requirement-build branch May 29, 2024 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Docs Docs and examples related
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

2 participants