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

Fix to version numbering so that it is in accordance with PEP 404 #456

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

eduardoscsouza
Copy link

Recently I tried installing this repo through pip, and it failed with the following message:

raise InvalidVersion(f"Invalid version: '{version}'")
pkg_resources.extern.packaging.version.InvalidVersion: Invalid version: 'b4eb09b4a3d167557d999723f7c2ed0d53a1bb7d-'

After some googling, I found that this is by design:

Apparently, Setuptools only supports version numbering in accordance with PEP 404 from version 66 onwards. So I created a fork of this repo, changing the version numbering scheme so that it can be installed through pip.

The idea is essentially the same as it is currently, with only some minor modifications. Currently, the version number is simply the hash of the most recent commit, so there isn't any pattern or sequence to be worried about. PEP 404 required the version number to be in base 10, so I changed the base 16 representation of the commit hash to base 10. By keeping the commit hash, the commit can still be retrieved through the version number. I also prepended the commit's timestamp, so that the more recent commits have higher version numbering.

@eduardoscsouza eduardoscsouza requested a review from a team as a code owner May 5, 2023 21:59
@google-ml-butler google-ml-butler bot added the size:S CL Change Size: Small label May 5, 2023
Copy link
Member

@MarkDaoust MarkDaoust left a comment

Choose a reason for hiding this comment

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

Thanks.

@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull labels May 5, 2023
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label May 5, 2023
@eduardoscsouza
Copy link
Author

Is anything else needed for this PR to be merged?

@markmcd
Copy link
Member

markmcd commented May 15, 2023

Is anything else needed for this PR to be merged?

It looks like this hasn't auto-merged because it's failing an internal lint check (lines are too long). I tried to update the PR but your branch is locked.

This is what I was trying to push:

$ git diff HEAD^
diff --git a/setup.py b/setup.py
index ba7cedde..3372dd02 100644
--- a/setup.py
+++ b/setup.py
@@ -35,9 +35,18 @@ project_name = 'tensorflow-examples'
 # The timestamp is used so that newer commits have a higher version number
 # The hash is used so that the specific commit can be identified
 # The hash integer can be converted back to the hash string with: '%032x' % commit_hash_int
-commit_hash_int = int(subprocess.check_output(['git', 'rev-parse', 'HEAD']).decode('utf-8').strip(), 16)
-commit_timestamp = subprocess.check_output(['git', 'show', '-s', '--format=%ct', 'HEAD']).decode('utf-8').strip()
-version = f"0.{commit_timestamp}.{commit_hash_int}"
+commit_hash_int = int(
+    subprocess.check_output(['git', 'rev-parse', 'HEAD'])
+    .decode('utf-8')
+    .strip(),
+    16,
+)
+commit_timestamp = (
+    subprocess.check_output(['git', 'show', '-s', '--format=%ct', 'HEAD'])
+    .decode('utf-8')
+    .strip()
+)
+version = f'0.{commit_timestamp}.{commit_hash_int}'
 
 if nightly:
   project_name = 'tensorflow-examples-nightly'

in non-diff format, that's

commit_hash_int = int(
    subprocess.check_output(['git', 'rev-parse', 'HEAD'])
    .decode('utf-8')
    .strip(),
    16,
)
commit_timestamp = (
    subprocess.check_output(['git', 'show', '-s', '--format=%ct', 'HEAD'])
    .decode('utf-8')
    .strip()
)
version = f'0.{commit_timestamp}.{commit_hash_int}'

@MarkDaoust
Copy link
Member

Thanks Mark. I'll fix it in the merge.

copybara-service bot pushed a commit that referenced this pull request May 16, 2023
…P 404

Please approve this CL. It will be submitted automatically, and its GitHub pull request will be marked as merged.

Imported from GitHub PR #456

Recently I tried installing this repo through pip, and it failed with the following message:

```
raise InvalidVersion(f"Invalid version: '{version}'")
pkg_resources.extern.packaging.version.InvalidVersion: Invalid version: 'b4eb09b4a3d167557d999723f7c2ed0d53a1bb7d-'
```

After some googling, I found that this is by design:

* pypa/setuptools#2497
* pypa/setuptools#3772

Apparently, `Setuptools` only supports version numbering in accordance with [PEP 404](https://peps.python.org/pep-0440/) from version 66 onwards. So I created a fork of this repo, changing the version numbering scheme so that it can be installed through pip.

The idea is essentially the same as it is currently, with only some minor modifications. Currently, the version number is simply the hash of the most recent commit, so there isn't any pattern or sequence to be worried about. PEP 404 required the version number to be in base 10, so I changed the base 16 representation of the commit hash to base 10. By keeping the commit hash, the commit can still be retrieved through the version number. I also prepended the commit's timestamp, so that the more recent commits have higher version numbering.

Copybara import of the project:

  - db7542d Change version numbering to be in accordance with PEP 440... by Eduardo <eduardoscsouza@gmail.com>
  - 94860f6 Minor changes for clarity and add explanatory comments. by Eduardo <eduardoscsouza@gmail.com>
  - f0d9155 Merge 94860f6 into b4eb0... by Eduardo Santos Carlos de Souza <eduardoscsouza@gmail.com>

PiperOrigin-RevId: 532326489
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to pull size:S CL Change Size: Small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants