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

Preceding v character from PEP 0440 #89

Closed
ghassanmas opened this issue Mar 22, 2023 · 7 comments
Closed

Preceding v character from PEP 0440 #89

ghassanmas opened this issue Mar 22, 2023 · 7 comments

Comments

@ghassanmas
Copy link

ghassanmas commented Mar 22, 2023

The util extract_version function is not ignoring a preceding 'v' when exists

  • Is this intentional ? hence in testing it expect to keep it

    ("v1.2.3 -- 2022-04-06", "v1.2.3"),

  • Following the linked PEP 0440 it says it should be removed ref

  • This would lead to not detecting if a version already exists in the entries because 'version' would be x.y.z while everion would be vx.y.z

    • This is true when; assuming v exists in the tempalte of the entry, e.g. the pattern followed in tutor
  • This would lead to scriv failing the following condition:

    if eversion == version:

scriv/src/scriv/util.py

Lines 53 to 72 in d86dc04

VERSION_REGEX = r"""(?ix) # based on https://peps.python.org/pep-0440/
\b # at a word boundary
v? # maybe a leading "v"
(\d+!)? # maybe a version epoch
\d+(\.\d+)+ # the meat of the version number: N.N.N
(?P<pre>
[-._]?[a-z]+\.?\d*
)? # maybe a pre-release: .beta3
([-._][a-z]+\d*)* # maybe post and dev releases
(\+\w[\w.]*\w)? # maybe a local version
\b
"""
def extract_version(text: str) -> Optional[str]:
"""Find a version number in a text string."""
m = re.search(VERSION_REGEX, text)
if m:
return m[0]
return None

@nedbat
Copy link
Owner

nedbat commented Mar 23, 2023

Can you give me more details about a specific failure case? PEP 440 says to ignore the leading "v" when comparing version numbers, but I would think anyone using scriv would either have a "v" everywhere, or have it nowhere. How is Tutor using versions numbers, and what problem is scriv causing?

@ghassanmas
Copy link
Author

The probelm comes when scriv not detecting that a version arleady exits in the chaneloge entries, the way to reproduce is

  • to have v in the entry template and then
  • you can use just he normal version format N.N.N.
  • running scriv create && scrive collect X times would lead X duplicate entries in changelog file
  • And lastly here is a sample feat: add scriv overhangio/tutor-forum#20 where I am adding scriv to python based package.

Regarding PEP 440, if I understood correctly for that PEP v.1.2.3 == 1.2.3, and then as I understood when normalizing a version it should be stripped.

When I used the referred package from the same PEP 440 it sripped the v

>>> from packaging import version
>>> version.parse('v1.2.3')
<Version('1.2.3')>
>>> version.parse('1.2.3')
<Version('1.2.3')>

@nedbat
Copy link
Owner

nedbat commented Mar 27, 2023

Why not put the "v" character into the __version__ string? Wouldn't it compare correctly then?

@ghassanmas
Copy link
Author

CC: @regisb

@regisb
Copy link

regisb commented Apr 6, 2023

In the context of Tutor: the __version__ string is used sometimes with and without a preceding "v". For instance, the Changelog titles include "v". But the releases that are pushed to pypi do not include "v". And in many places of the tutor code base, we make use of the Tutor version without the preceding "v".

So we can't really include the preceding "v" in tutor.about.__version__. We can however modify the scriv configuration to include the prefix in the {{ version }}. We would have to concatenate a literal with a string -- is this possible?

Alternatives include:

  • Ignoring this problem: the fact that we are generating alternative versions is really not a big deal.
  • Have scriv parse the id attribute of the <a id='changelog-X.Y.Z'></a> field from the changelog instead of the title.
  • Make sure that the prefix is stripped from the VERSION_REGEX, for instance using a named expression.

Sidenote: while writing this issue, I realized that the {{ version }} that is included in a GitHub release template is not the same as the scriv version, but it is sampled from the changelog title -- so it does include the "v" prefix. Is there a reason why the scriv github-release command is not fetching the version from the scriv configuration file?

@nedbat
Copy link
Owner

nedbat commented Apr 16, 2023

This is now released as part of scriv 1.3.1.

Is there a reason why the scriv github-release command is not fetching the version from the scriv configuration file?

Yes, so that it can be used even on projects that don't use scriv to create their changelog (like coverage.py).

@nedbat nedbat closed this as completed Apr 16, 2023
@regisb
Copy link

regisb commented Apr 17, 2023

Awesome, thanks!

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

No branches or pull requests

3 participants