-
-
Notifications
You must be signed in to change notification settings - Fork 25.9k
MNT use x.y.z (no rc/beta/etc) for version in sphinx config #15566
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
Conversation
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.
The binder stuff below requires 'dev' to be identifiable in version
. But it should probably rather be using release
.
I did the simplest thing at the time. I agree this is not very robust and this may cause complications with the beta release. You may have discussed this kind of things already elsewhere: rather than Also the
There could also be an intermediary step where you install |
'Ill-formed version: {!r}. Version should follow ' | ||
'PEP440'.format(version)) | ||
|
||
if v.is_devrelease: |
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.
Maybe I am missing something, but I think pkg_resources.parse_version
does not return an object with a .is_devrelease
attribute. On the other hand, packaging.version.parse
does.
In [1]: from packaging.version import parse
In [2]: parse('0.22.dev0').is_devrelease
Out[2]: True
In [3]: from pkg_resources import parse_version
In [4]: parse_version('0.22.dev0').is_devrelease
---------------------------------------------------------------------------
AttributeError Traceback (most recent call last)
<ipython-input-4-13d9ab197353> in <module>
----> 1 parse_version('0.22.dev0').is_devrelease
AttributeError: 'Version' object has no attribute 'is_devrelease'
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 rather not have a dependency just to parse the version. And the issue is that the new pkg_resources
actually do have the is_devrelease
attribute, and it works fine. The older ones however, don't. Trying to figure out when it was added to it.
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.
On the other hand, setuptools actually ships a version of packaging
with it.
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.
🤦♂️ setuptools
ships packaging=16.8
, whereas Version().is_devrelease
was introduced in packaging=17.0
. It worked on my system because pkg_resources
uses the installed lib instead of the vendored one if it finds it, and I had a new packaging
in that environment.
all green \o/ But now we're installing |
For now this seems to be the only blocker for an RC by the way. |
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.
So we will add a packaging
dependency for building the docs, which can be removed if setuptools
update its dependency?
I am okay with this. Thank you @adrinjalali !
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.
Hmmmm... Okay. I'd rather avoid the dependency but it's maybe no big deal
me too, but since our docs have quite a few deps anyway, and this dependency will be removed when setuptools upgrade their vendored version of packaging, thought it's not big deal. |
…rn#15566) * use x.y.z (no rc/beta/etc) for version in sphinx config * use parse_version, and refactor binder branch inference * install packaging in doc build envs * use packaging
We expect the
version
to bex.y.z
(no rc/beta/etc), andrelease
to include the beta/rc/etc info. This PR takes the base part of the__version__
forversion
variable in sphinx config.Note that the regexp checkng for this had been introduced in #11221
Alternatively we can fix that regexp.