trio icon indicating copy to clipboard operation
trio copied to clipboard

Some publicly exposed functions/properties have no documentation

Open A5rocks opened this issue 9 months ago • 4 comments

This issue should only really be looked at after https://github.com/python-trio/trio/issues/3221 is closed (because this is mainly about what people see in an editor, rather than our documentation). The following attributes are listed in our _type_completeness.json as not having a docstring:

  • [ ] trio._unix_pipes.FdStream.send_all
  • [ ] trio._unix_pipes.FdStream.wait_send_all_might_not_block
  • [ ] trio._unix_pipes.FdStream.receive_some
  • [ ] trio._unix_pipes.FdStream.close
  • [ ] trio._unix_pipes.FdStream.aclose
  • [ ] trio._unix_pipes.FdStream.fileno
  • [ ] trio._core._io_epoll._EpollStatistics
  • [ ] trio._channel.MemoryReceiveChannel
  • [ ] trio._channel.MemoryChannelStatistics
  • [ ] trio._channel.MemorySendChannel
  • [ ] trio._core._run.Task
  • [ ] trio._socket.SocketType
  • [ ] trio._highlevel_socket.SocketStream.send_all
  • [ ] trio._highlevel_socket.SocketStream.wait_send_all_might_not_block
  • [ ] trio._highlevel_socket.SocketStream.send_eof
  • [ ] trio._highlevel_socket.SocketStream.receive_some
  • [ ] trio._highlevel_socket.SocketStream.aclose
  • [ ] trio._subprocess.HasFileno.fileno
  • [ ] trio._sync.AsyncContextManagerMixin
  • [ ] trio._sync._HasAcquireRelease.acquire
  • [ ] trio._sync._HasAcquireRelease.release
  • [ ] trio._sync._LockImpl
  • [ ] trio._core._local._NoValue
  • [ ] trio._core._local.RunVarToken
  • [ ] trio.socket.gaierror
  • [ ] trio.socket.herror
  • [ ] trio._core._mock_clock.MockClock.start_clock
  • [ ] trio._core._mock_clock.MockClock.current_time
  • [ ] trio._core._mock_clock.MockClock.deadline_to_sleep_time
  • [ ] trio.testing._raises_group._ExceptionInfo.exconly
  • [ ] trio.testing._raises_group._ExceptionInfo.errisinstance
  • [ ] trio.testing._raises_group._ExceptionInfo.getrepr
  • [ ] trio.testing._raises_group.RaisesGroup.expected_type

I expect most of these are trivial. There's a couple groups I noticed while copying them over:

  • some aren't actually publicly exposed and so we should update check_type_completeness.py to filter them out (e.g. _HasAcquireRelease)
  • some should really just inherit documentation from parents (e.g. SocketStream.wait_send_all_might_not_block). I don't know if pyright supports anything like that but it should

I think some of the logic from check_type_completeness.py is kind of strange: it shouldn't filter out docstrings that are available at runtime because... pyright still can't see those?

A5rocks avatar Mar 20 '25 20:03 A5rocks

I think some of the logic from check_type_completeness.py is kind of strange: it shouldn't filter out docstrings that are available at runtime because... pyright still can't see those?

So what it does is:

  1. Check if pyright sees the docstring
  2. if pyright doesn't see the docstring, try to runtime import it a. if we fail to runtime import it.. it's usually for OS-specific reasons, so we ignore a bunch of objects here
  3. check if it has a docstring at runtime

If neither pyright nor runtime sees the docstring (and it's not Path) then it emits an error.

It might be that comments/docstrings need clarification, but I don't think the logic is wrong.

jakkdl avatar Mar 31 '25 09:03 jakkdl

I think that's misguided. Like it makes sense but accomplishes an incorrect goal. Either the goal is:

  • everything public has a docstring when you hover over it in an editor (<- I was assuming this previously)
  • the docs are up-to-date

In the first case we shouldn't care about if the docstrings are available at runtime, I think. (I don't use VSCode so I can't say whether it gets docstrings at runtime, but I assume not.) In the second, we now have a better check so we can disable this. I suppose the current logic makes sense if we're trying to accomplish "you can call help() on all public symbols and all referenced symbols" but I think that test could probably be written down better.

A5rocks avatar Mar 31 '25 09:03 A5rocks

Hah, I actually read a lot of docstrings in the live prompt with help() when coding (as I don't have hover-to-get-docstring configured). Ultimately the current logic came from "oh pyright is complaining about a bunch of stuff that in fact does have docstrings, it just fails to see them for complicated reasons"

But it sounds like the test should be rewritten to not put both of those in the same bucket, since we both want to support static tools and help(). And then we have the sphinx logic for making sure docs is good.

jakkdl avatar Mar 31 '25 10:03 jakkdl

Hah, I actually read a lot of docstrings in the live prompt with help() when coding (as I don't have hover-to-get-docstring configured).

But it sounds like the test should be rewritten to not put both of those in the same bucket, since we both want to support static tools and help().

Likewise, I find using help() invaluable, and I agree.

CoolCat467 avatar Mar 31 '25 14:03 CoolCat467