Skip to content

Make regex methods accept all buffer types as inputs #7158

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.

Sign up for GitHub

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

Closed
wants to merge 32 commits into from
Closed

Make regex methods accept all buffer types as inputs #7158

Changes from 2 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
3344316
Make `mmap.mmap` extend `bytearray`
itaisteinherz Feb 7, 2022
6039732
Extend `ReadableBuffer` instead of `bytearray`
itaisteinherz Feb 8, 2022
74aa464
Fixup
itaisteinherz Feb 8, 2022
8666c61
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Feb 8, 2022
fdd90cd
Change `AnyStr` to include `ByteString` instead of `bytes`
itaisteinherz Feb 8, 2022
634c867
Merge branch 'feature/mmap-bytearray' of https://github.com/itaistein…
itaisteinherz Feb 8, 2022
efe5ee0
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Feb 8, 2022
6978ce6
Revert unwanted changes
itaisteinherz Feb 14, 2022
24e2a34
Finish reverting
itaisteinherz Feb 14, 2022
b9b1176
Fix PoC
itaisteinherz Feb 14, 2022
2ded7aa
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Feb 14, 2022
eb0e953
Attempt to create custom type for regex strings
itaisteinherz Feb 14, 2022
38f5996
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Feb 14, 2022
1a56af6
Fixup
itaisteinherz Feb 14, 2022
e26ef4e
Fix regex string typevar
itaisteinherz Feb 14, 2022
884af92
Update `Match` to use `RegexString`
itaisteinherz Feb 14, 2022
c9af022
Fix extended typevar
itaisteinherz Feb 14, 2022
6e320ae
Attempt to fix
itaisteinherz Feb 14, 2022
d3094f9
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Feb 14, 2022
d775b51
Remove unused imports
itaisteinherz Feb 14, 2022
d97f24a
Merge branch 'feature/mmap-bytearray' of https://github.com/itaistein…
itaisteinherz Feb 14, 2022
c63f691
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Feb 14, 2022
ee5573a
Fix linting
itaisteinherz Feb 14, 2022
6d470e2
Merge branch 'feature/mmap-bytearray' of https://github.com/itaistein…
itaisteinherz Feb 14, 2022
85a5004
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Feb 14, 2022
46af759
Ignore linting error
itaisteinherz Feb 14, 2022
d9e68ac
Fixup old changes
itaisteinherz Feb 14, 2022
0fe7ca3
Revert last changes
itaisteinherz Feb 18, 2022
c4f23f7
Add second typevar argument to `Match`
itaisteinherz Feb 19, 2022
e26ab0c
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Feb 19, 2022
a410b16
`re.match` with `mmap.mmap` PoC
itaisteinherz Feb 19, 2022
7314338
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Feb 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion stdlib/mmap.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ if sys.platform != "win32":

PAGESIZE: int

class mmap(AbstractContextManager[mmap], Iterable[int], Sized):
class mmap(AbstractContextManager[mmap], ReadableBuffer, Iterable[int], Sized):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Another thought: how does this even work? ReadableBuffer is a Union which includes mmap.mmap, which leads me to the conclusion that this should result in some sort of infinite type loop.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Update: this does lead to an error :) Will probably split up WriteableBuffer to fix this.

Copy link
Member

Choose a reason for hiding this comment

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

We should add it to the union, not use the union as a base class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you mean? mmap.mmap is already part of WriteableBuffer.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, right. Then we shouldn't change it at all.

The original problem was that mmap wasn't accepted in re.match. If re.match accepts all buffers, then we should change its signature, not anything about mmap.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it. Will look into re.match. Note that having mmap.mmap extend ReadOnlyBuffer does make some sense to me, though you're probably right.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Upon further research, it seems to me like a way to solve this would be to have mmap.mmap extend ByteString, and change the definition of AnyStr to:

AnyStr = TypeVar("AnyStr", str, ByteString)

However, that would require a change in CPython. I could also create an AnyStr alternative, specific to Pattern, with this definition. One last option would be to have mmap.mmap extend bytes (aka ReadOnlyBuffer). What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

We should not change AnyStr. That would require CPython runtime changes. We could use a separate typevar specific to regexes, but I'd have to look more into it to figure out whether that's feasible.

if sys.platform == "win32":
def __init__(self, fileno: int, length: int, tagname: str | None = ..., access: int = ..., offset: int = ...) -> None: ...
else:
Expand Down