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.
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
pattern match on base version in Version constructor #41
pattern match on base version in Version constructor #41
Changes from 2 commits
5890bdf
cdb9417
9729c62
d8e9eba
49017ea
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Could you explain the use case for this? I didn't really think anyone would need to turn a version string back into a
Version
instance.If we do add it, I'd probably prefer it to be a separate function, like
def parse_version(version: str) -> Version
.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.
It's use full when you want to modify a version that is gathered from
importlib.metadata
. Sincedunamai
just grabs the string, everything is put insidebase
.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.
Ah, that's a great point.
Okay, let's separate this from the constructor and put it as
Version.parse(cls, version: str, pattern: str = _VERSION_PATTERN) -> "Version"
. That way, the constructor stays simple and predictable.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.
I've pushed an possible implementation.
Version.parse
allows a version as well so that you can use the function as follows:dunamai.Version.parse(dunamai.get_version("dunamai", third_choice=dunamai.Version.from_any_vcs))
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 keep
parse()
simpler, I think I'll just add aparser
argument toget_version()
after the merge.parser=Version
would keep the existing behavior, andparser=Version.parse
would enable the new behavior.