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

MAINT: exprtype regex simplify #18074

Merged
merged 1 commit into from
Dec 27, 2020

Conversation

tylerjereddy
Copy link
Contributor

  • determineexprtype_re_1 was modified to remove extraneous
    character class markers around a single ,

  • a similar change was applied for the word metacharacter
    in determineexprtype_re_2 and determineexprtype_re_3

  • the third character class in determineexprtype_re_3 was
    simplified to remove an escape sequence--by placing the
    - at the start of the character class its metacharacter
    status can be avoided

* `determineexprtype_re_1` was modified to remove extraneous
character class markers around a single `,`

* a similar change was applied for the word metacharacter
in `determineexprtype_re_2` and `determineexprtype_re_3`

* the third character class in `determineexprtype_re_3` was
simplified to remove an escape sequence--by placing the
`-` at the start of the character class its metacharacter
status can be avoided
determineexprtype_re_3 = re.compile(
r'\A[+-]?[\d.]+[\d+\-de.]*(_(?P<name>[\w]+)|)\Z', re.I)
r'\A[+-]?[\d.]+[-\d+de.]*(_(?P<name>\w+)|)\Z', re.I)
Copy link
Member

Choose a reason for hiding this comment

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

I argue that moving - to be beginning of the set is marginally less readable, one more thing to remember :) But OK.

@charris charris merged commit f5f845b into numpy:master Dec 27, 2020
@charris
Copy link
Member

charris commented Dec 27, 2020

Thanks Tyler. The CircleCI tests seem to be running on your repo, do you have them enabled there?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants