Added argument to param.Version to support dev versions #200
Conversation
|
I'll definitely add enough unit tests to make sure test coverage increases instead of decreasing. |
param/version.py
Outdated
| @@ -233,6 +243,7 @@ def __str__(self): | |||
| """ | |||
| if self.release is None: return 'None' | |||
| release = '.'.join(str(el) for el in self.release) | |||
| release = '%sdev%d' % (release, self.dev) if self.dev else release | |||
There was a problem hiding this comment.
Shouldn't there be a "." before "dev" by PEP440?
There was a problem hiding this comment.
I think it is fine, this is a snippet Bryan posted:
In [1]: from packaging.version import Version as V
In [2]: V("0.12.10dev3")
Out[2]: <Version('0.12.10.dev3')>
In [3]: v = V("0.12.10dev3")
In [4]: v.is_prerelease
Out[4]: True
In [5]: v.base_version
Out[5]: '0.12.10'
In [6]: v > V("0.12.11")
Out[6]: FalseThat said, I am happy to consider adding the dot - even though our existing dev tags don't have the dot, they have all been lightweight tags so far (and therefore have no effect on param.Version).
@philippjfr Do you want the dot?
There was a problem hiding this comment.
If we aren't worried about people referencing the old tags (I'm not - they are dev versions!) then I could go back and replace the old dev tags with the new format.
There was a problem hiding this comment.
Alternatively, we can delete the tags for old dev versions that have been followed by a release. Then we can use the new format going forwards.
There was a problem hiding this comment.
I'd prefer to leave out the dot personally, bokeh does too.
|
Looks fine by me apart from comment above. |
|
I am also ok with this. Thanks a lot for doing it. Despite the jokes, I appreciate it. (I will try to come up with new versions of the jokes.) Seems unfortunate that it's necessary to support both with and without the dot
Examples off the top of my head... No dot:
Dot:
|
|
I added a pep440 flag we can use to stick to the |
param/version.py
Outdated
| @@ -233,6 +249,8 @@ def __str__(self): | |||
| """ | |||
| if self.release is None: return 'None' | |||
| release = '.'.join(str(el) for el in self.release) | |||
| release = ('%s%sdev%d' % (release, '.' if self.pep440 else '', self.dev) | |||
| if self.dev else release) | |||
There was a problem hiding this comment.
Do people do dev0 (thinking along the lines of 0 being false above)? Actually, I realize now I have seen e.g. 1.x.dev (numba, pandas). But we can't support everything, and I think just .dev doesn't comply with PEP 440 ("The developmental release segment consists of the string .dev, followed by a non-negative integer value").
This is all getting a bit crazy, sorry...
|
I think I'll go with a suggestion from @jbednar - support tags with and without the dot and return the version string with the dot. Then you have the flexibility for tagging while using the PEP440 compliant version strings. |
|
Looks ok to me, thanks :) |
Added
devargument that allows dev versions to be handled like regular release versions. I'm happy to add more unit tests to make sure everyone is satisfied with this addition.