-
Notifications
You must be signed in to change notification settings - Fork 168
chore: Bump version manually only in pyproject.toml #2514
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
Merged
Merged
Changes from 19 commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
777495e
chore: Bump version manually only in pyproject.toml
FBruzzesi 8485943
rollback pre-commit
FBruzzesi e213fcf
workaround via makefile
FBruzzesi ed06050
use uv version bump instead of manual process
FBruzzesi a130984
do we have uv?
FBruzzesi 50efe76
install uv to begin with
FBruzzesi 59ea2d2
ok easier than expected
FBruzzesi 4200ffc
eureka - add a python env initializer
FBruzzesi 49e1dc1
Merge remote-tracking branch 'upstream/main' into chore/less-version-β¦
dangotbanned 79d7a89
chore(ruff): lint
dangotbanned 6636d60
ci: Add some constants
dangotbanned 8a79221
ci: alias `subprocess` to `sp`
dangotbanned 47192ae
rollback to avoid using importlib metadata version
FBruzzesi 798a6dd
pre-commit typo entry
FBruzzesi 71996c3
chore(suggestion): Lazy `__version__`
dangotbanned 07f61c5
update bump_version and simplify
FBruzzesi e29949b
Merge branch 'main' into chore/less-version-manual-parsing
FBruzzesi 205f38a
simplify tags deletion
FBruzzesi 6fba9ba
Update .pre-commit-config.yaml
FBruzzesi 2a77e7b
Merge branch 'main' into chore/less-version-manual-parsing
dangotbanned File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| from __future__ import annotations | ||
|
|
||
| import re | ||
| from pathlib import Path | ||
|
|
||
| import narwhals as nw | ||
|
|
||
|
|
||
| def test_version_matches_pyproject() -> None: | ||
| with Path("pyproject.toml").open(encoding="utf-8") as file: | ||
| content = file.read() | ||
| pyproject_version = re.search(r'version = "(.*)"', content).group(1) # type: ignore[union-attr] | ||
|
|
||
| assert nw.__version__ == pyproject_version |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,80 +1,61 @@ | ||
| # mypy: ignore | ||
| # ruff: noqa | ||
| import re | ||
| import subprocess | ||
| from __future__ import annotations | ||
|
|
||
| import subprocess as sp | ||
| import sys | ||
|
|
||
| out = subprocess.run(["git", "fetch", "upstream", "--tags"]) | ||
| GIT = "git" | ||
| UV = "uv" | ||
| FETCH = "fetch" | ||
| PUSH = "push" | ||
| COMMIT = "commit" | ||
| UPSTREAM = "upstream" | ||
| TAGS = "--tags" | ||
| TAG = "tag" | ||
| VERSION = "version" | ||
|
|
||
| out = sp.run([GIT, FETCH, UPSTREAM, TAGS], check=False) | ||
| if out.returncode != 0: | ||
| print( | ||
| "Something went wrong with the release process, please check the Narwhals Wiki and try again." | ||
| "Something went wrong with the release process, please check the Narwhals Wiki for " | ||
| "at https://github.com/narwhals-dev/narwhals/wiki#release-process and try again." | ||
| ) | ||
| print(out) | ||
| sys.exit(1) | ||
| subprocess.run(["git", "reset", "--hard", "upstream/main"]) | ||
| sp.run([GIT, "reset", "--hard", "upstream/main"], check=False) | ||
|
|
||
| if ( | ||
| subprocess.run( | ||
| ["git", "branch", "--show-current"], text=True, capture_output=True | ||
| current_branch := sp.run( | ||
| [GIT, "branch", "--show-current"], text=True, capture_output=True, check=False | ||
| ).stdout.strip() | ||
| != "bump-version" | ||
| ): | ||
| msg = "`bump_version.py` should be run from `bump-version` branch" | ||
| ) != "bump-version": | ||
| msg = f"`bump_version.py` should be run from `bump-version` branch instead of `{current_branch}`" | ||
| raise RuntimeError(msg) | ||
|
|
||
| # Delete local tags, if present | ||
| try: | ||
| # Get the list of all tags | ||
| result = subprocess.run( | ||
| ["git", "tag", "-l"], capture_output=True, text=True, check=True | ||
| ) | ||
| result = sp.run([GIT, TAG, "-l"], capture_output=True, text=True, check=True) | ||
| tags = result.stdout.splitlines() # Split the tags into a list by lines | ||
|
|
||
| # Delete each tag using git tag -d | ||
| for tag in tags: | ||
| subprocess.run(["git", "tag", "-d", tag], check=True) | ||
| if tags: | ||
| # Delete each tag using git tag -d | ||
| sp.run([GIT, TAG, "-d", *tags], check=True) | ||
|
|
||
| print("All local tags have been deleted.") | ||
| except subprocess.CalledProcessError as e: | ||
| except sp.CalledProcessError as e: | ||
| print(f"An error occurred: {e}") | ||
|
|
||
| subprocess.run(["git", "fetch", "upstream", "--tags"]) | ||
| subprocess.run(["git", "fetch", "upstream", "--prune", "--tags"]) | ||
| sp.run([GIT, FETCH, UPSTREAM, TAGS], check=False) | ||
| sp.run([GIT, FETCH, UPSTREAM, "--prune", TAGS], check=False) | ||
|
|
||
| how = sys.argv[1] | ||
|
|
||
| with open("pyproject.toml", encoding="utf-8") as f: | ||
| content = f.read() | ||
| old_version = re.search(r'version = "(.*)"', content).group(1) # pyright: ignore[reportOptionalMemberAccess] | ||
| version = old_version.split(".") | ||
| if how == "patch": | ||
| version = ".".join(version[:-1] + [str(int(version[-1]) + 1)]) | ||
| elif how == "minor": | ||
| version = ".".join(version[:-2] + [str(int(version[-2]) + 1), "0"]) | ||
| elif how == "major": | ||
| version = ".".join([str(int(version[0]) + 1), "0", "0"]) | ||
| content = content.replace(f'version = "{old_version}"', f'version = "{version}"') | ||
| with open("pyproject.toml", "w", encoding="utf-8") as f: | ||
| f.write(content) | ||
|
|
||
| with open("narwhals/__init__.py", encoding="utf-8") as f: | ||
| content = f.read() | ||
| content = content.replace( | ||
| f'__version__ = "{old_version}"', | ||
| f'__version__ = "{version}"', | ||
| ) | ||
| with open("narwhals/__init__.py", "w", encoding="utf-8") as f: | ||
| f.write(content) | ||
|
|
||
| with open("docs/installation.md", encoding="utf-8") as f: | ||
| content = f.read() | ||
| content = content.replace( | ||
| f"'{old_version}'", | ||
| f"'{version}'", | ||
| ) | ||
| with open("docs/installation.md", "w", encoding="utf-8") as f: | ||
| f.write(content) | ||
| new_version = sp.run( | ||
| [UV, VERSION, "--bump", how, "--short"], capture_output=True, text=True, check=False | ||
| ).stdout | ||
|
|
||
| subprocess.run(["git", "commit", "-a", "-m", f"release: Bump version to {version}"]) | ||
| subprocess.run(["git", "tag", "-a", f"v{version}", "-m", f"v{version}"]) | ||
| subprocess.run(["git", "push", "upstream", "HEAD", "--follow-tags"]) | ||
| subprocess.run(["git", "push", "upstream", "HEAD:stable", "-f", "--follow-tags"]) | ||
| sp.run([GIT, COMMIT, "-a", "-m", f"release: Bump version to {new_version}"], check=False) | ||
| sp.run([GIT, TAG, "-a", f"v{new_version}", "-m", f"v{new_version}"], check=False) | ||
| sp.run([GIT, PUSH, UPSTREAM, "HEAD", "--follow-tags"], check=False) | ||
| sp.run([GIT, PUSH, UPSTREAM, "HEAD:stable", "-f", "--follow-tags"], check=False) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
To be completely honest I'd rather just update in two places, I have no idea what this is doing
Happy to take the installation update in the docs page though π
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.
Just to make sure I understand correctly - the concern with importing
importlib.metadatais regarding startup time? see commentThere 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 feel uneasy about the global - do you have a reference from another project that does this?
Uh oh!
There was an error while loading. Please reload this page.
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.
Looking for
__version__ = version(__name__)on github you will get many resultsA couple of projects I know that made it in the first few pages of the list are:
Edit: some other project do the analogous of
__version__ = version("narwhals")(but couldn't find something I know)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.
sure, thanks
still not sure we need it though - there's some other nice improvements in here though, happy to take those
regarding perf,
from narwhals import __version__currently takes 0m0.156s, andfrom importlib import metadatatakes 0m0.108sUh oh!
There was an error while loading. Please reload this page.
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.
Some notes:
__version__doesn't seem to be mentioned as a standard in the python docs__getattr__is a well-defined method of deferring imports3.14reinlocaleΒ python/cpython#129860IMO, I don't think there's a use-case for checking
narwhals.__version__, but if people want to do that - the cost of it isn't paid by everyone else since we're providing it lazily.Are there any search results of a package checking it? (sorry I'm replying from my phone π )
@MarcoGorelli I understand your caution and ultimately will defer π to you, but I think this balances:
Uh oh!
There was an error while loading. Please reload this page.
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.
thanks - could you elaborate on the
globalpart please? i've had a look at the linked projects and they don't seem to have itif we don't deem
__version__to be useful to most users, presumably we don't need theglobaleither?Uh oh!
There was an error while loading. Please reload this page.
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.
Sure @MarcoGorelli!
I'm assuming you're referring to the projects in (#2514 (comment))?
The 3
stdlibexamples I gave in (#2514 (comment)) all featured use ofglobal:I guess for some more info on
global, these might be helpfulAs a more concrete example, we can do the following on
main:But in this PR, we can see where the
__getattr__hook slips in:Without the
global:__version__is never added tonw.__dict__nw.__version__always goes via__getattr__, which is triggered after the regular__dict__lookup failsSo the
globalis what provides the lazy import, since the work is only ever done once and only when a user asks for it π.All subsequent lookups behave identically to a regular lookup into the namespace.
As another example,
polarshas 2 behaviors related to__getattr__:pl.NUMERIC_DTYPESThere 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.
Interestingly,
polarsseems to be fine with usingfind_specand still considers that lazy