Skip to content

Simplify tag_regex a bit#185

Open
MridulS wants to merge 1 commit intomainfrom
fix_tag_regex
Open

Simplify tag_regex a bit#185
MridulS wants to merge 1 commit intomainfrom
fix_tag_regex

Conversation

@MridulS
Copy link
Member

@MridulS MridulS commented Mar 9, 2026

I think the more strict regex seems to create issues on certain systems. This should also make it easier to debug.

[tool.setuptools_scm]
root = "../.."
tag_regex = "^essimaging/(?P<version>[vV]?\\d+(?:\\.\\d+)*(?:[._-]?\\w+)*)$"
tag_regex = "^essimaging/(?P<version>.*)$"
Copy link
Member

Choose a reason for hiding this comment

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

Not sure it should be this loose. Can you give an example why we can't have stricter requirements on the tag? At least something like ^essimaging/(?P<version>[vV]?\d+.*)$ to ensure the tag starts with a number (or v`).

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm fine with that too. I just wanted to remove the escapes as much as possible. I'm not sure if these things affects different systems differently?

Copy link
Member

Choose a reason for hiding this comment

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

Which escapes are you talking about?

Copy link
Member Author

Choose a reason for hiding this comment

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

The double \\

Copy link
Member

Choose a reason for hiding this comment

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

^essimaging/(?P<version>[vV]?\\d+.*)$

Like this right?

We decided not to use v so shouldn't it be like this?

^essimaging/(?P<version>\\d+.*)$

Copy link
Member

Choose a reason for hiding this comment

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

The double \

This should be independent of the OS. It's Python-specific.

^essimaging/(?P<version>[vV]?\\d+.*)$

Like this right?

Yes.

We decided not to use v so shouldn't it be like this?

No harm allowing a v IMHO. But if we want to be strict, then yes, we can leave it out.

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.

3 participants