Skip to content

Conversation

kklein
Copy link
Collaborator

@kklein kklein commented Jul 28, 2022

Before:
Screenshot 2022-07-28 at 6 16 11 PM

After:
Screenshot 2022-07-28 at 6 16 32 PM

@codecov
Copy link

codecov bot commented Jul 28, 2022

Codecov Report

Merging #43 (a49e730) into main (2152b5b) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main      #43   +/-   ##
=======================================
  Coverage   93.75%   93.75%           
=======================================
  Files          15       15           
  Lines        1586     1586           
=======================================
  Hits         1487     1487           
  Misses         99       99           
Impacted Files Coverage Δ
src/datajudge/requirements.py 94.50% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us.

@kklein kklein changed the title Attempt to use type hints for parameters in documentation. Minor improvements on readthedocs Jul 29, 2022
@kklein kklein marked this pull request as ready for review August 1, 2022 18:59
@ivergara
Copy link
Collaborator

ivergara commented Aug 2, 2022

Why would you say it's an improvement to not have the types in the signature but in a separate block instead?

@kklein
Copy link
Collaborator Author

kklein commented Aug 2, 2022

@ivergara I acknowledge that in principle it is difficult to make a priori statements about superiority in terms of UI matters without user tests.

The upside I personally saw with the sphinx_autodoc_typehints change was that I find it easier to parse what the parameters of a function/method/class are if they are displayed as a list as compared to a sequence of words.

Afaict, this list representation is often achieved by having a dedicated parameters block as part of the doc string, e.g. [0].

My thought was that we could gain the advantage of having a similar representation without having to rewrite all of our docstrings by using this very sphinx extension.

[0] https://github.com/Quantco/glum/blob/main/src/glum/_glm.py#L126-L132

@ivergara
Copy link
Collaborator

ivergara commented Aug 2, 2022

The upside I personally saw with the sphinx_autodoc_typehints change was that I find it easier to parse what the parameters of a function/method/class are if they are displayed as a list as compared to a sequence of words.

I do have to agree that it's easier to parse a vertical than a horizontal list.

@kklein kklein merged commit 1432d1b into main Aug 2, 2022
@kklein kklein deleted the doc_typehints branch August 2, 2022 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants