sphinx icon indicating copy to clipboard operation
sphinx copied to clipboard

Unify docutils type annotations

Open danieleades opened this issue 1 year ago • 9 comments

there are two sources of truth for docutils type annotations.

this repo uses docutils-stubs, but there's also typeshed annotations - types-docutils.

Neither are complete, nor is one a subset of the other. types-docutils was historically pretty poor but has seen more active development lately and has improved. doctuils-stubs is more complete, but is out of date and unmaintained.

I believe this repo should migrate to the typeshed annotations and upstream anything missing from docutils-stubs into typeshed.

cc @tk0miya

  • [x] https://github.com/python/typeshed/pull/11468
  • [ ] https://github.com/python/typeshed/pull/11469
  • [ ] https://github.com/python/typeshed/pull/11471
  • [x] https://github.com/python/typeshed/pull/11473
  • [x] https://github.com/python/typeshed/pull/11481
  • [x] https://github.com/python/typeshed/pull/11490
  • [x] https://github.com/python/typeshed/pull/11492
  • [x] https://github.com/sphinx-doc/sphinx/pull/12034
  • [ ] https://github.com/python/typeshed/pull/11523
  • [ ] https://github.com/python/typeshed/pull/11521
  • [x] https://github.com/python/typeshed/pull/11468
  • [x] https://github.com/python/typeshed/pull/10102
  • [x] https://github.com/python/typeshed/pull/10099
  • [ ] https://github.com/sphinx-doc/sphinx/pull/12042
  • [ ] https://github.com/python/typeshed/pull/11524
  • [ ] https://github.com/python/typeshed/pull/11525
  • [ ] https://github.com/python/typeshed/pull/11526
  • [ ] https://github.com/python/typeshed/pull/11530

and the actual PR to migrate to typeshed (WIP): https://github.com/sphinx-doc/sphinx/pull/12012

danieleades avatar Feb 25 '24 18:02 danieleades

Have started tweaking type annotations in typeshed, with a view to reach parity and then transition this repo to typeshed

https://github.com/python/typeshed/pull/11468

danieleades avatar Feb 25 '24 21:02 danieleades

Good idea! I am always frustrated because of how poor autocompletion is done with docutils on Pycharm so I'll be very happy if this can be merged upstream.

picnixz avatar Feb 26 '24 08:02 picnixz

i've created a PR to track the migration here - https://github.com/sphinx-doc/sphinx/pull/12012

contributions (both to the PR and upstream in typeshed) very very welcome!

danieleades avatar Feb 26 '24 09:02 danieleades

@sphinx-doc/triagers @sphinx-doc/developers @sphinx-doc/former-maintainers @picnixz

can I pretty please get a solid review of https://github.com/python/typeshed/pull/11469?

It introduces a generic type argument to the docutils state machine to represent the context. That generic argument will propogate through into a massive number of docutils types so is a significant change. Before that happens I want to check this is correct! I'm not super familiar with the inner workings of the docutils statemachines. Is the 'context' truly generic? what is its type?

from the docutils docs:

context: application-specific storage.

danieleades avatar Mar 03 '24 12:03 danieleades

I won't be there next week, and I don't think I'll have time for that today. But I'll do it by in two weeks (I will take time for that).

picnixz avatar Mar 03 '24 13:03 picnixz

@sphinx-doc/triagers @sphinx-doc/developers @sphinx-doc/former-maintainers @picnixz

can I pretty please get a solid review of python/typeshed#11469?

It introduces a generic type argument to the docutils state machine to represent the context. That generic argument will propogate through into a massive number of docutils types so is a significant change. Before that happens I want to check this is correct! I'm not super familiar with the inner workings of the docutils statemachines. Is the 'context' truly generic? what is its type?

from the docutils docs:

context: application-specific storage.

stand down!

I've postponed adding generics, even if they are technically correct. The decision is blocking progress and would have significant downstream impact. I'll revisit in a separate PR and just type the context as Any for now

danieleades avatar Mar 03 '24 21:03 danieleades

Is the 'context' truly generic?

If you are talking about state transitions, like def blank(self, match, context, next_state):, then context is a list[str]

I'm not super familiar with the inner workings of the docutils statemachines

I am, I've been through it in excruciating detail, because I'm writing an RST parser as we speak 😉

This all looks great, thanks for all the work! ... although obviously it would be a million time easier, if we could just type docutils itself 😅

chrisjsewell avatar Mar 03 '24 22:03 chrisjsewell

If you are talking about state transitions, like def blank(self, match, context, next_state):, then context is a list[str]

I believe that's true of the rst-specific subclasses. Is it also true of the superclass? (I'll take your word for it)

danieleades avatar Mar 04 '24 05:03 danieleades

Ah thank you chris for helping here since I am travelling (I'm currently in a train). Could I ask you to help with the upstream PRs (I can come back later if there are some questions though it's been a while since I played with docutils)?

picnixz avatar Mar 04 '24 06:03 picnixz

Could I ask you to help with the upstream PRs

Okie, I will try to carve out some time for it 👍

chrisjsewell avatar Mar 05 '24 09:03 chrisjsewell

i'm proposing that https://github.com/sphinx-doc/sphinx/pull/12012 be merged, warts and all so that all future improvements to typing are made against the community-supported typeshed docutils stubs

danieleades avatar Mar 21 '24 11:03 danieleades