-
-
Notifications
You must be signed in to change notification settings - Fork 18.3k
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: Loading sphinxcontrib.spelling to sphinx only if it's available (#21396) #21397
DOC: Loading sphinxcontrib.spelling to sphinx only if it's available (#21396) #21397
Conversation
Good call: I agree that it is an optional dependency. You don't need it to install |
ci/requirements-optional-conda.txt
Outdated
@@ -20,6 +20,7 @@ pytest-xdist | |||
s3fs | |||
scipy | |||
seaborn | |||
sphinxcontrib-spelling |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these should go in requirements_dev.txt instead (only)
@@ -9,3 +9,4 @@ python-dateutil>=2.5.0 | |||
pytz | |||
setuptools>=24.2.0 | |||
sphinx | |||
sphinxcontrib-spelling |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this file indicates it is autogenerated
As said on the issue, given the installation troubles, I would make this an optional one. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apart from the comment where to put the dep, looks good to me!
@@ -13,3 +13,4 @@ dependencies: | |||
- pytz | |||
- setuptools>=24.2.0 | |||
- sphinx | |||
- sphinxcontrib-spelling |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should rather put it in the requirements-pip/conda-optional.txt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jreback, you said it should be in requirements_dev.txt
, may be you want to explain why? I also thought the optional files made more sense, but you two surely know better than me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
requirements_dev are for pure dev things
but i think things have gotten confused
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, let's discuss that in another one then
Just FYI, I just followed the instructions to create a development environment with Conda. On the
Is that desired behavior or should the |
git diff upstream/master -u -- "*.py" | flake8 --diff
I think the optional dependencies go directly into the
.txt
files for pip and conda, and not into the yaml file. Please correct me if I'm wrong.Seems like besides
sphinxcontrib-spelling
, a binary programenchant
is required, and it's not available in conda for what I've seen. Not sure what's the right approach for it.