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

Add miniver to simplify development branch descriptions #726

Closed
wants to merge 9 commits into from

Conversation

hmaarrfk
Copy link
Contributor

Closes: #696

pip install -e . -vv
# You might only need to run this once I believe. we don't run it daily on our dev machines
In [1]: import pygfx

In [2]: pygfx.__version__
'0.1.18.post29+g9342016'

In [3]: from packaging.version import Version

In [4]: Version('0.1.18.post29+g9342016') > Version('0.1.18')
True

In [5]: assert Version('0.1.18.post29+g9342016') > Version('0.1.18')

In [6]: assert Version('0.1.18.post29+g9342016') < Version('0.1.19')

So I use this in my project and the github action that I use to upload the package to pypi will make the static version correctly.

You don't need to change the source when you create a new release. just tag then your CI will pickup the new version in the upload to pypi.

see: https://github.com/ramonaoptics/python-teensytoany
Xref: https://github.com/jbweston/miniver

@hmaarrfk hmaarrfk requested a review from Korijn as a code owner April 13, 2024 13:56
__version__ = "0.1.18"
version_info = tuple(map(int, __version__.split(".")))
from ._version import __version__
version_info = tuple(map(int, __version__.split(".")[:3]))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure what behavior you want from this version_info tuple. I don't have such constructs in my code.

I typically use

from packaging import Version

for all my comparisons.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's do this:

Suggested change
version_info = tuple(map(int, __version__.split(".")[:3]))
version_info = tuple(int(v) if v.isnumeric() else v for v in __version__.split("."))

@hmaarrfk hmaarrfk marked this pull request as draft April 13, 2024 13:59
@hmaarrfk
Copy link
Contributor Author

oh man, the CIs made this more difficult than I wanted to.

.github/workflows/ci.yml Outdated Show resolved Hide resolved
```
In [1]: import pygfx

In [2]: pygfx.__version__
'0.1.18.post29+g9342016'

In [3]: from packaging.version import Version

In [4]: Version('0.1.18.post29+g9342016') > Version('0.1.18')
True

In [5]: assert Version('0.1.18.post29+g9342016') > Version('0.1.18')

In [6]: assert Version('0.1.18.post29+g9342016') < Version('0.1.19')
```

So I use this in my project and the github action that I use to upload
the package to pypi will make the static version correctly

see: https://github.com/ramonaoptics/python-teensytoany
@hmaarrfk
Copy link
Contributor Author

hmm, i've never used pyinstaller ...

@hmaarrfk
Copy link
Contributor Author

This is actually the first time i read through the miniver code in earnest.

I understand how it is designed now, and I somewhat understand their design decisions of having 2 files.

But it is somewhat overly complicated.

It may be interesting to use https://github.com/proudust/gh-describe though this seems like getting more untrusted code in the repo.

Copy link
Collaborator

@almarklein almarklein left a comment

Choose a reason for hiding this comment

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

I'm not overly enthusiastic 😅 but this may also be because I don't understand the problem that it solves. Let's discuss in #696

__version__ = "0.1.18"
version_info = tuple(map(int, __version__.split(".")))
from ._version import __version__
version_info = tuple(map(int, __version__.split(".")[:3]))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's do this:

Suggested change
version_info = tuple(map(int, __version__.split(".")[:3]))
version_info = tuple(int(v) if v.isnumeric() else v for v in __version__.split("."))

Comment on lines +49 to +50
p = subprocess.Popen(
["git", "describe", "--long", "--always", "--tags", "--dirty"] + opts,
Copy link
Collaborator

Choose a reason for hiding this comment

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

IIUC this means that if someone obtains pygfx in another way than via a package manager or git repo, the version cannot be established.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct. There become some "edge cases" that you have to start taking into account when people "receive" the source code.

I just checked my project for example, the version downloaded from pypi burns things in due to the modifications to the setup.py but the archive from github would make it pretty difficult for people to use.

image

@Korijn
Copy link
Collaborator

Korijn commented Apr 15, 2024

My general feeling about this is that it's adding a ton of code, complexity and dependencies to solve a very small simple manual step... not too enthusiastic.

@hmaarrfk
Copy link
Contributor Author

complexity and dependencies [...] not too enthusiastic.

Understood. This is a fair criticism.

@hmaarrfk hmaarrfk closed this Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Would you consider miniver for version management?
3 participants