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

typings: Fixes and improvements for dbcore.query #4847

Merged
merged 2 commits into from
Jul 18, 2023

Conversation

wisp3rwind
Copy link
Member

The next iteration of introducing typings into dbcore. This is splitting out only dbcore.query; with some additional changes, I have clean mypy on dbcore, but that'd be a bit too much for a single PR.

unintentionally, the pattern normalization added in
1c3a053 was a no-op, and only value
normalization was actually applied (since self.pattern was left
unchanged)

This also helps with typing by ensuring that variables have fixed types
Copy link
Member

@sampsyo sampsyo left a comment

Choose a reason for hiding this comment

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

Excellent! This looks right everywhere, according to my "brain type checker."

except re.error as exc:
# Invalid regular expression.
raise InvalidQueryArgumentValueError(pattern,
"a regular expression",
format(exc))

def col_clause(self):
super().__init__(field, pattern_re, fast)
Copy link
Member

Choose a reason for hiding this comment

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

[non-actionable] The diff confused me for a sec here, so for future reviewers: here's where the super call moved to.

else:
raise ValueError("pattern must be bytes, str, or memoryview")

super().__init__(field, bytes_pattern)
Copy link
Member

Choose a reason for hiding this comment

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

Another solid improvement to the old version, where the mutation was admittedly hard to follow.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's really just pleasing mypy, which does not like shadowing of variables at all.

@sampsyo sampsyo merged commit 7b86f61 into beetbox:master Jul 18, 2023
@wisp3rwind wisp3rwind deleted the dbcore_typing_3 branch July 18, 2023 19:10
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.

2 participants