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

Remaining tasks until --verifytypes passes and check_type_completeness.py can be removed #2734

Closed
41 tasks done
jakkdl opened this issue Aug 3, 2023 · 20 comments · Fixed by #2877
Closed
41 tasks done
Labels
typing Adding static types to trio's interface

Comments

@jakkdl
Copy link
Member

jakkdl commented Aug 3, 2023

To get a bit of an overview of the remaining files until we hit Type Completeness ™️

Public API

Possibly more files if running with a --pythonplatform other than linux.

Private or deprecated (lower priority)

May include files that should be in the above list, I didn't look through files in a fully complete way

Tests

Typing the tests will check that it's possible to use the interface in a reasonable way, as well as catch any possible bugs in the tests themself of course.

  • trio/testing/_fake_net
  • trio/_core/_tests/
  • trio/_tests/

docstrings

Cleanup

  • Remove check_type_completeness and verify_types_*.json, minor fixes #2877
    • Remove references to py.typed in the code, from when we needed to generate it on-the-fly because it wasn't statically added in the package.
    • Remove check_type_completeness.py, verify_types_[windows,linux,darwin].json and their entries in check.sh
    • Add pyright --verifytypes with multiple platforms to check.sh, and/or pre-commit.
@CoolCat467 CoolCat467 added the typing Adding static types to trio's interface label Aug 3, 2023
@CoolCat467
Copy link
Member

_core/_asyncgens in #2735

@A5rocks
Copy link
Contributor

A5rocks commented Aug 5, 2023

FYI if you aren't aware, members of the org can edit issue comments! (I'm just saying, it would be cool to have people edit the initial comment in addition to their comment for notifications, so @jakkdl doesn't have to edit this all the time :-)

@jakkdl
Copy link
Member Author

jakkdl commented Aug 6, 2023

Clarified [_generated]_xxx into two separate listings for _xxx and a subheader for _generated_xxx.

@jakkdl
Copy link
Member Author

jakkdl commented Aug 6, 2023

Note that _multierror, _asyncgens, etc were intentionally left out of my listing as they're not required for the public interface - which should be the first priority. But I'll add a second list for non-public untyped files so those files can be coordinated on as well, since we do eventually want everything typed to catch any bugs in trio itself.

@jakkdl
Copy link
Member Author

jakkdl commented Aug 6, 2023

FYI if you aren't aware, members of the org can edit issue comments! (I'm just saying, it would be cool to have people edit the initial comment in addition to their comment for notifications, so @jakkdl doesn't have to edit this all the time :-)

Please make a comment when non-trivially editing the list though, so everybody gets a notification

@jakkdl
Copy link
Member Author

jakkdl commented Aug 8, 2023

Collapsed testing/, and added #2720 as a TODO

@jakkdl
Copy link
Member Author

jakkdl commented Aug 10, 2023

Ran --verifytypes on windows & darwin and split out files needed for those systems, where I noticed I missed a couple docstrings in #2743

EDIT: it seems I misplaced this edit, but handled in #2762

@jakkdl
Copy link
Member Author

jakkdl commented Aug 14, 2023

Noting down two small TODOS that weren't fully addressed in #2682

@jakkdl
Copy link
Member Author

jakkdl commented Aug 21, 2023

Checked off a couple dozen tasks, though didn't bother linking them to PRs. Also added a task for adding TypeVarTuple

@jakkdl
Copy link
Member Author

jakkdl commented Aug 23, 2023

I've started working on SocketType, and associated tests: test_highlevel_open_tcp_stream, test_highlevel_open_tcp_listeners, test_highlevel_socket, test_socket

@A5rocks
Copy link
Contributor

A5rocks commented Aug 24, 2023

For what it's worth, I don't think it's completely worth our time to completely type tests. I can imagine there's quite some dynamism going on there :P

I imagine everyone's got an opinion on this already, but maybe we can try just typing a few easy ones (no insane dynamic stuff) and try to aim for typing a breadth of usage (rather than comprehensively for every test)?

@TeamSpen210
Copy link
Contributor

Fully dynamic stuff no, indeed the main benefit is having a lot of sample usages of all the various functions to confirm the types make sense.

@jakkdl
Copy link
Member Author

jakkdl commented Aug 25, 2023

For what it's worth, I don't think it's completely worth our time to completely type tests. I can imagine there's quite some dynamism going on there :P

I imagine everyone's got an opinion on this already, but maybe we can try just typing a few easy ones (no insane dynamic stuff) and try to aim for typing a breadth of usage (rather than comprehensively for every test)?

Yeah we could set up a separate set of settings for test files, or just be more lax with type: ignore and Any

@A5rocks
Copy link
Contributor

A5rocks commented Oct 4, 2023

Uh, well we have py.typed so I assume this issue is solely about typing the tests now, right?

@CoolCat467
Copy link
Member

Uh, well we have py.typed so I assume this issue is solely about typing the tests now, right?

As far as I know, this is correct, except that _windows_pipes was still untyped, but #2812 is fixing that now.

@jakkdl
Copy link
Member Author

jakkdl commented Oct 6, 2023

yeah the title of the issue is definitely a misnomer, and with the excitement of adding types dying down as we're down to just the chore of typing the test files we don't really need this issue as a tracker either (the blacklist in pyproject.toml achieves it in a better way).

There's some cleanup that needs to be done though, there's 27 references to py.typed/py_typed which should be possible to remove now, and if anybody does anything with #2762 then we can fully get rid of check_type_completeness, the json output files and directly run pyright --verifytypes in CI.

@A5rocks
Copy link
Contributor

A5rocks commented Nov 4, 2023

Bump once again to make sure this is still relevant now that test files are typed too. (thanks again!)

@jakkdl
Copy link
Member Author

jakkdl commented Nov 4, 2023

There's some cleanup that needs to be done though, there's 27 references to py.typed/py_typed which should be possible to remove now, and if anybody does anything with #2762 then we can fully get rid of check_type_completeness, the json output files and directly run pyright --verifytypes in CI.

This is still the current status, I'll rename the issue.

@jakkdl jakkdl changed the title Remaining files until --verifytypes passes and we can add py.typed Remaining files until --verifytypes passes and check_type_completeness.py can be removed Nov 4, 2023
@jakkdl jakkdl changed the title Remaining files until --verifytypes passes and check_type_completeness.py can be removed Remaining tasks until --verifytypes passes and check_type_completeness.py can be removed Nov 4, 2023
@jakkdl
Copy link
Member Author

jakkdl commented Nov 4, 2023

Updated the tasks in the OP

@jakkdl
Copy link
Member Author

jakkdl commented Nov 17, 2023

🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
typing Adding static types to trio's interface
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants