[DISCUSS] Versioninfo tuples

[kevin-bates]

This issue is being opened so that we can finish a discussion started in #324 (comment) so as to not detract from the pull request's primary objective.

The last couple of comments between Jason and myself were in regards to this change recently made in jupyter core: https://github.com/jupyter/jupyter_core/blob/master/jupyter_core/version.py

personally found that to be overly complex

It could have been way more complex for full pep440 compliance, but we dialed it back :).

The purpose for the extra stuff in that file is to provide a good versioninfo tuple that mirrors the versioninfo tuple from Python more faithfully, and to make it so that you don't have to think about how pep440 formats version numbers (dot? consistent abbreviation?). I think it probably does that as simply as possible, but (a) if there is something simpler, I'd love to see it - I think it's a bit much to stomach as well, and (b) of course, if you don't want those two goals, that's a different story.

cc: @jasongrout

[kevin-bates]

of course, if you don't want those two goals, that's a different story.

I'm not sure where this is coming from, but I hope you know that I believe version tuples should be properly formatted. This is why my update earlier today includes a reference to the applicable section of PEP440. I'm hoping maintainers will read that when building alpha, beta, or candidate releases, although I suspect a simple comment will suffice as well.

I completely agree with your intention. I think one simplification to your approach would be to just include the dot in the value for the 'dev' key, then rework the 'if/else' block, leaving:

version_info = VersionInfo(4, 8, 0, 'dev', 0)

_specifier_ = {'alpha': 'a', 'beta': 'b', 'candidate': 'rc', 'final': '', 'dev': '.dev'}

_suffix_ = ''
if version_info.releaselevel != 'final':
    _suffix_ = f'{_specifier_[version_info.releaselevel]}{version_info.serial}'

__version__ = f'{version_info.major}.{version_info.minor}.{version_info.micro}{_suffix_}'

With this approach, I find myself referring to the structure definition, the specifier and the templated values and am left with the thought of whether I trust my eyes - that's all.

In polling the 16 Jupyter-ecosystem forks I have (not all of which are up to date), only jupyterlab and jupyter_core go to these extents (although lab doesn't address dev builds). Of the rest, very few use a dotted dev suffix, in violation of PEP440, so this is fairly widespread.

[jasongrout]

I'm not sure where this is coming from, but I hope you know that I believe version tuples should be properly formatted. This is why my update earlier today includes a reference to the applicable section of PEP440. I'm hoping maintainers will read that when building alpha, beta, or candidate releases, although I suspect a simple comment will suffice as well.

Sorry, I didn't mean to sound like I was judging here. It's a tradeoff, and I think it's your call as a primary maintainer where you draw the line about complexity in the code vs how much burden it is on the user to remember details about updating (like putting that period before 'dev') vs how compliant we are to pep440 or the conventions python itself uses. I don't think there's a definite right or wrong decision here.

[jasongrout]

I think one simplification to your approach would be to just include the dot in the value for the 'dev' key, then rework the 'if/else' block, leaving:

That's a really nice simplification!

[kevin-bates]

Sorry, I didn't mean to sound like I was judging here.

I'm sorry I misinterpreted the original comment and appreciate your response.

or the conventions python itself uses

Since I'm still relatively new to python, do you happen to have an example of this or a reference to where this comes into play? I see both version_info and __version__, mostly defined together but sometimes one or the other, and I'd like to better understand the expectations of consumers.

[goanpeca]

I think it is pretty safe to assume a __version__ in a package. As a python dev and user I expect this to be at the top level package. This is almost now a convention.

Version info is a nice thing but I would only consider the former "mandatory". This is a nicer way for devs and user to check compatibility with numbers being ints, but in libraries that only provide the former you would split on dots and do the thing yourself. Not that terrible :)

Now I think none of these are mandatory, just well agreed upon conventions. The former is a stronger one, the latter a not so strong one.

My 2 cents.

[jasongrout]

First, I agree 100% with what @goanpeca said.

Here is what Python itself provides, which we took as a pattern for the versioninfo thing we wrote for jlab: https://docs.python.org/3/library/sys.html#sys.version_info

[kevin-bates]

Thank you Jason and Gonzalo - those were helpful responses. I think it's sufficient to leave things as they are now (simple with links to the PEP). Should we find confusion and mistakes when building dev and pre-releases or find integrations requiring richer version_info support, we can consider a more explicit and robust mechanism. That said, if someone feels strongly otherwise, I'm okay making changes.

Closing.

[dhirschfeld]

Is there any reason you wouldn't just use the canonical packaging implementation to parse the version string?

In [2]: from packaging.version import Version

In [3]: Version('3.2.1.post001+gdsdfsa5')
Out[3]: <Version('3.2.1.post1+gdsdfsa5')>

In [4]: Version('3.2.1.post001+gdsdfsa5') > Version('3.2.1.dev001+gdsdfsa5')
Out[4]: True

In [5]: Version('3.2.1dev')
Out[5]: <Version('3.2.1.dev0')>

In [6]: Version('3.2.1.dev001+gdsdfsa5')
Out[6]: <Version('3.2.1.dev1+gdsdfsa5')>

[kevin-bates]

Thanks for this information @dhirschfeld - it's all a learning experience for me!

I really like how Version() "fixes" things up and knows when a separator should be inserted prior to the suffix, the proper serial value, etc.

So _version.py would look something like this?

from packaging import Version

version_info = Version('1.1.0.dev')
__version__ = version_info.public

where folks could work with version_info where they need that granularity and __version__ would be '1.1.0.dev0'.

[jasongrout]

That's great! Someone has already implemented all the tricky parts in a standardized package!

[dhirschfeld]

For my part, I'd be inclined to just include a PEP-440 compatible version string in __version__.

I wouldn't worry about also including a Version object because if a user wants that to e.g. robustly compare versions they can simply import the class from packaging.version themselves and if they don't need that then doing it for them is just an unnecessary cost which will increase import times.

[jasongrout]

I wouldn't worry about also including a Version object because if a user wants that to e.g. robustly compare versions they can simply import the class from packaging.version themselves and if they don't need that then doing it for them is just an unnecessary cost which will increase import times.

Very good point. Now that there is a simple standard way to convert pep-440 compatible version strings into more user-friendly version objects, having a version info object is not nearly as important.