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

fix missing sphinx testing dependencies #1751

Merged
merged 7 commits into from
Apr 3, 2024
Merged

Conversation

drammock
Copy link
Collaborator

cc @trallard this should fix the defusedxml errors. It was added as a new optional dependency to sphinx within the last week. It seems like we were already getting all the other sphinx [test] deps in other ways (just by luck) so strictly speaking I could have just added [test] to the sphinx dev URL, but why leave it fragile if we don't have to.

@drammock drammock added kind: maintenance Improving maintainability and reducing technical debt tag: CI Pull requests that update GitHub Actions code labels Mar 29, 2024
Copy link
Collaborator

@trallard trallard left a comment

Choose a reason for hiding this comment

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

Nice to know I did not break anything 😔 thanks for looking into this and sending a fix.

Will still work on improving our CI/tests in the next few days

@drammock
Copy link
Collaborator Author

argh. things that work locally aren't working when run via the action. Will revisit next week, but if anyone knows the right combination of ", ', and { just by looking, feel free to comment/push here

@trallard
Copy link
Collaborator

I can sort it out when I am back at work on Tuesday.

@drammock
Copy link
Collaborator Author

drammock commented Apr 2, 2024

OK I think I got it @trallard. The 3.12 CI is still failing, but it's failing at the codecov upload step, not when trying to import defusedxml when running the tests.

fi
set -x # print commands
python -m pip install --upgrade pip wheel setuptools
python -m pip install -e .["$1"] ${SPHINX_INSTALL} $2 $DEP_EXTRA
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

FYI $DEP_EXTRA is probably cruft from whoever we copied this file from. It's never used by us.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We can leave it for now 🤷🏽‍♀️ the whole thing needs an overhaul

@trallard
Copy link
Collaborator

trallard commented Apr 3, 2024

Let's merge this as is since the codecov bit is the missing one atm

@trallard trallard merged commit f18e6ec into pydata:main Apr 3, 2024
15 of 16 checks passed
@drammock drammock deleted the sphinx-deps branch April 3, 2024 18:03
ivanov pushed a commit to ivanov/pydata-sphinx-theme that referenced this pull request Jun 5, 2024
* fix missing sphinx testing dependencies

* do the specifier correctly

* fix quoting

* fix again

* try another way

* try quoting again

* try without whitespace
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: maintenance Improving maintainability and reducing technical debt tag: CI Pull requests that update GitHub Actions code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants