Fixes for strict-bytes #10454

Dreamsorcerer · 2025-02-11T15:51:32Z

This will be default behaviour in mypy 2, so best to fix it early.

It would be good if anybody else can check over these, the existing code seems a little confused with when it wants bytes only or will allow bytes/bytearray/memoryview.

for more information, see https://pre-commit.ci

aiohttp/abc.py

aiohttp/compression_utils.py

aiohttp/http_writer.py

aiohttp/web_response.py

aiohttp/web_ws.py

for more information, see https://pre-commit.ci

Dreamsorcerer · 2025-02-11T16:03:36Z

I'm unclear if the remaining errors are a bug with us or with typeshed (which says the method doesn't support memoryview[bytes]).

codecov · 2025-02-11T16:05:00Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.69%. Comparing base (7d77e12) to head (5b3337a).
Report is 3 commits behind head on master.

✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #10454   +/-   ##
=======================================
  Coverage   98.69%   98.69%           
=======================================
  Files         122      122           
  Lines       37225    37230    +5     
  Branches     2063     2064    +1     
=======================================
+ Hits        36740    36745    +5     
  Misses        338      338           
  Partials      147      147

Flag	Coverage Δ
CI-GHA	`98.57% <95.65%> (-0.01%)`	⬇️
OS-Linux	`98.24% <95.65%> (-0.01%)`	⬇️
OS-Windows	`96.17% <95.65%> (-0.01%)`	⬇️
OS-macOS	`97.36% <95.65%> (+<0.01%)`	⬆️
Py-3.10.11	`97.26% <91.30%> (-0.01%)`	⬇️
Py-3.10.16	`97.81% <91.30%> (-0.01%)`	⬇️
Py-3.11.11	`97.89% <91.30%> (-0.01%)`	⬇️
Py-3.11.9	`97.35% <91.30%> (-0.01%)`	⬇️
Py-3.12.8	`97.14% <86.95%> (?)`
Py-3.12.9	`98.33% <86.95%> (-0.03%)`	⬇️
Py-3.13.1	`97.14% <86.95%> (-0.01%)`	⬇️
Py-3.13.2	`98.31% <86.95%> (-0.01%)`	⬇️
Py-3.9.13	`97.14% <91.30%> (-0.01%)`	⬇️
Py-3.9.21	`97.68% <91.30%> (-0.01%)`	⬇️
Py-pypy7.3.16	`97.28% <91.30%> (+9.17%)`	⬆️
VM-macos	`97.36% <95.65%> (+<0.01%)`	⬆️
VM-ubuntu	`98.24% <95.65%> (-0.01%)`	⬇️
VM-windows	`96.17% <95.65%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

codspeed-hq · 2025-02-11T16:05:07Z

CodSpeed Performance Report

Merging #10454 will not alter performance

_{Comparing strict-bytes (5b3337a) with master (7d77e12)}

Summary

✅ 47 untouched benchmarks

asvetlov · 2025-02-11T17:18:14Z

Would you backport the PR

Dreamsorcerer · 2025-02-11T17:36:36Z

Would you backport the PR

It changes the (apparent) API, so I'd prefer to avoid it. Hopefully will be focusing on v4 after the typing work I'm doing.

asvetlov · 2025-02-11T19:50:15Z

Up to you, any decision is good to me.

asvetlov · 2025-02-11T19:54:48Z

Yes, focusing on V4 would be good.

Personally, I should review the diff and file a list of required changes related to my commits on master that were not backported.
I have quite a tight schedule this month; but the task is in my to-do list.

Dreamsorcerer · 2025-02-15T00:37:50Z

I'm unclear if the remaining errors are a bug with us or with typeshed (which says the method doesn't support memoryview[bytes]).

@bdraco I think you've played with that code fairly recently, any idea if memoryview[bytes] is actually supported?

bdraco · 2025-02-20T19:02:48Z

asyncio Transport write is typed to (method) def write(data: bytes | bytearray | memoryview[int]) -> Any

so I think only memoryview[int] is needed.

Dreamsorcerer · 2025-02-20T20:01:39Z

asyncio Transport write is typed to (method) def write(data: bytes | bytearray | memoryview[int]) -> Any

so I think only memoryview[int] is needed.

We have tests that explicitly use memoryview[bytes] though. So, there's either a bug in our code, or in typeshed.

bdraco · 2025-02-20T20:15:51Z

I couldn't find any tests where we have memoryview[bytes] .. AFAICT memoryview(b"") has a type of memoryview[int]....

def memoryview_bytes(x: "memoryview[bytes]") -> None:
    pass


def memoryview_int(x: "memoryview[int]") -> None:
    pass


v = memoryview(b"")

memoryview_int(v)  # pass

memoryview_bytes(
    v
)  # error: Argument 1 to "memoryview_bytes" has incompatible type "memoryview[int]"; expected "memoryview[bytes]"  [arg-type]

Dreamsorcerer · 2025-02-20T20:30:01Z

I couldn't find any tests where we have memoryview[bytes] .. AFAICT memoryview(b"") has a type of memoryview[int]....

If you drop bytes from the union, then mypy will flag it up.

Dreamsorcerer · 2025-02-20T20:37:59Z

tests/test_http_writer.py:454:21: error: Argument 1 to "write" of
"StreamWriter" has incompatible type "memoryview[bytes]"; expected
"bytes | bytearray | memoryview[int]"  [arg-type]
        await msg.write(payload)
                        ^~~~~~~
tests/test_http_writer.py:473:21: error: Argument 1 to "write" of
"StreamWriter" has incompatible type "memoryview[bytes]"; expected
"bytes | bytearray | memoryview[int]"  [arg-type]
        await msg.write(payload)
                        ^~~~~~~

aiohttp/tests/test_http_writer.py

Lines 442 to 477 in 982ed47

    
           async def test_write_payload_2d_shape_memoryview( 
        
               buf: bytearray, 
        
               protocol: BaseProtocol, 
        
               transport: asyncio.Transport, 
        
               loop: asyncio.AbstractEventLoop, 
        
           ) -> None: 
        
               msg = http.StreamWriter(protocol, loop) 
        
               msg.enable_chunking() 
        
               mv = memoryview(b"ABCDEF") 
        
               payload = mv.cast("c", [3, 2]) 
        
               await msg.write(payload) 
        
               await msg.write_eof() 
        
               thing = b"6\r\nABCDEF\r\n0\r\n\r\n" 
        
               assert thing == buf 
        
           async def test_write_payload_slicing_long_memoryview( 
        
               buf: bytearray, 
        
               protocol: BaseProtocol, 
        
               transport: asyncio.Transport, 
        
               loop: asyncio.AbstractEventLoop, 
        
           ) -> None: 
        
               msg = http.StreamWriter(protocol, loop) 
        
               msg.length = 4 
        
               mv = memoryview(b"ABCDEF") 
        
               payload = mv.cast("c", [3, 2]) 
        
               await msg.write(payload) 
        
               await msg.write_eof() 
        
               thing = b"ABCD" 
        
               assert thing == buf

bdraco · 2025-02-20T20:44:44Z

payload = mv.cast("c", [3, 2])
That's what I was missing

def memoryview_bytes(x: "memoryview[bytes]") -> None:
    pass


def memoryview_int(x: "memoryview[int]") -> None:
    pass


v = memoryview(b"")

memoryview_int(v)  # pass

payload = v.cast("c", [3, 2])
memoryview_bytes(payload)

passes now

bdraco · 2025-02-20T20:47:18Z

tests/test_http_writer.py:454:21: error: Argument 1 to "write" of
"StreamWriter" has incompatible type "memoryview[bytes]"; expected
"bytes | bytearray | memoryview[int]"  [arg-type]
        await msg.write(payload)
                        ^~~~~~~
tests/test_http_writer.py:473:21: error: Argument 1 to "write" of
"StreamWriter" has incompatible type "memoryview[bytes]"; expected
"bytes | bytearray | memoryview[int]"  [arg-type]
        await msg.write(payload)
                        ^~~~~~~
aiohttp/tests/test_http_writer.py

Lines 442 to 477 in 982ed47

async def test_write_payload_2d_shape_memoryview(

buf: bytearray,

protocol: BaseProtocol,

transport: asyncio.Transport,

loop: asyncio.AbstractEventLoop,

) -> None:

msg = http.StreamWriter(protocol, loop)

msg.enable_chunking()

mv = memoryview(b"ABCDEF")

payload = mv.cast("c", [3, 2])

await msg.write(payload)

await msg.write_eof()

thing = b"6\r\nABCDEF\r\n0\r\n\r\n"

assert thing == buf

async def test_write_payload_slicing_long_memoryview(

buf: bytearray,

protocol: BaseProtocol,

transport: asyncio.Transport,

loop: asyncio.AbstractEventLoop,

) -> None:

msg = http.StreamWriter(protocol, loop)

msg.length = 4

mv = memoryview(b"ABCDEF")

payload = mv.cast("c", [3, 2])

await msg.write(payload)

await msg.write_eof()

thing = b"ABCD"

assert thing == buf

So it seems like we support it but asyncio is typed to only memoryview[int] since we end up calling write or writelines at

aiohttp/aiohttp/http_writer.py

Line 106 in 36aa738

transport.write(b"".join(chunks))

aiohttp/aiohttp/http_writer.py

Line 108 in 36aa738

transport.writelines(chunks)

def writelines(list_of_data: Iterable[bytes | bytearray | memoryview[int]]) -> None
def write(data: bytes | bytearray | memoryview[int]) -> None

I guess it works though since #4890 seems to use it

Dreamsorcerer · 2025-02-20T20:49:17Z

I guess it works though since #4890 seems to use it

Yeah, so typeshed bug then, I'll file a PR there.

bdraco · 2025-02-20T20:52:27Z

It looks like it gets cast to c (char)

aiohttp/aiohttp/http_writer.py

Line 129 in 36aa738

chunk = chunk.cast("c")

but c is still memoryview[bytes]...

So maybe asyncio only supports memoryview[int], but memoryview[bytes] happens to work and could break anytime....

Dreamsorcerer · 2025-02-28T14:31:09Z

Typeshed updated to memoryview[object].

Fixes for strict-bytes

156eef5

Dreamsorcerer requested review from webknjaz and asvetlov as code owners February 11, 2025 15:51

[pre-commit.ci] auto fixes from pre-commit.com hooks

4bec8c7

for more information, see https://pre-commit.ci