Add stubtest, fix incorrect stubs #73
Conversation
Zac-HD
left a comment
Zac-HD
left a comment
There was a problem hiding this comment.
One comment below, but this looks like a nice improvement over the status quo to me!
(I'd love for someone to look through the allowlist and decide whether we need any further fixes, but IMO that should not block this PR)
| def __init__( | ||
| self, exceptions: Sequence[BaseException], *, _collapse: bool = ... | ||
| ): ... | ||
| def __new__( | ||
| cls: T, exceptions: Sequence[BaseException], *, _collapse: bool = ... | ||
| ) -> T: ... |
There was a problem hiding this comment.
Probably better to not expose this param in stubs, since it's got a leading underscore and use of MultiError is deprecated anyway!
There was a problem hiding this comment.
Hmm, it is just a stub - and not an official API/documentation. If there weren't a stub file, type checkers would see the parameters, and in error messages and the like you could see references to it. So I think I'm leaning towards keeping it.
I think having fewer lines in the allowlist also is a good thing.
There was a problem hiding this comment.
I meant "have a stub for MultiError, which omits the _collapse parameter", I think we agree on the first part but I miscommunicated about the second?
There was a problem hiding this comment.
Ah no that's how I interpreted you 😅 So the miscommunication must be on my part! :D
I think adding trio.MultiError.__init__ and trio.MultiError.__new__ to allowlist.txt isn't great. Another reason for keeping the stubs technically correct is to ease a transition away from having separate stub files at all, and if exposing _collapse is bad for some reason that should be addressed in trio - rather than hidden by having an "incorrect" type for the function in the stub.
2 participants
Noticed due to
open_nurserymissing thestrict_exception_typesargument, but various other issues raised by stubtest also addressed to the best of my ability. If any of them are problematic, it's easy to chuck them into the allowlist instead.It's very possible that a bunch of the missing stubs in the allowlist should be added to the stubs.
I have no clue why
ci.shcreates an empty directory and symlinks inCHECK_TYPING, but whatever the reason I did the same forCHECK_STUBS.