-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
[aiofiles] .name
on NamedTempFile
is always a str
#13621
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
base: main
Are you sure you want to change the base?
Conversation
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
@@ -34,7 +33,7 @@ class _UnknownAsyncTextIO(AsyncBase[str]): | |||
@property | |||
def newlines(self) -> str | tuple[str, ...] | None: ... | |||
@property | |||
def name(self) -> FileDescriptorOrPath: ... | |||
def name(self) -> str: ... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we can do this unconditionally. aiofiles.threadpool.open()
will also return an AsyncTextIOWrapper
, which might be a file descriptor:
import asyncio
from os import fdopen
from aiofiles import open as aioopen
async def main():
f = open("bar.txt")
async with aioopen(f.fileno()) as aiof:
print(type(aiof.name)) # <class 'int'>
asyncio.run(main())
Maybe it's best if we make _UnknownAsyncTextIO
and the two wrapper classes generic over name
with a default of FileDescriptorOrPath
. Then we can bind this to str
in the return type of NamedTemporaryFile
.
In fact, we could use this as a template for the stdlib, where we already have a comment alluding to this:
Lines 771 to 774 in 0b8dcfc
# Usually str, but may be bytes if a bytes path was passed to open(). See #10737. | |
# If PEP 696 becomes available, we may want to use a defaulted TypeVar here. | |
@property | |
def name(self) -> str | Any: ... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great idea! Thanks a lot! ❤️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now, when I double checked that, I think that I would prefer to close this PR, because it is too complex. Notice that def NamedTemporaryFile
has multiple @overload
s that return different subtypes of _UnknownAsyncBinaryIO
, not even direct ones. I think that making all of the generic is an overkill :(
Closes #13551
Basically, this is a wrapper around
typeshed/stdlib/tempfile.pyi
Line 219 in 28106bc