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

Documentation: signatures with typehints. #638

Open
pfebrer opened this issue Nov 1, 2023 · 9 comments
Open

Documentation: signatures with typehints. #638

pfebrer opened this issue Nov 1, 2023 · 9 comments

Comments

@pfebrer
Copy link
Contributor

pfebrer commented Nov 1, 2023

Function signatures with typehints in the documentation can become very hard to read:

Screenshot from 2023-11-01 17-50-32

I been playing with some CSS, and it is possible to have each parameter in a new line, just as you would do in the source code:

Screenshot from 2023-11-01 18-11-13

Do you think it would be better?

@pfebrer
Copy link
Contributor Author

pfebrer commented Nov 1, 2023

You can tune it further to make it even clearer by adding colors for example:

Screenshot from 2023-11-01 18-20-48

@pfebrer
Copy link
Contributor Author

pfebrer commented Nov 1, 2023

Or even with the weight of the font, here I avoid setting the typehint to bold:

Screenshot from 2023-11-01 18-23-45

So, really you can do anything you come up with.

The way I did it applies the styling to methods as well, but I'm sure there's some way to avoid it:

Screenshot from 2023-11-01 18-23-38

@pfebrer
Copy link
Contributor Author

pfebrer commented Nov 1, 2023

Perhaps we can do a poll on discord to see which layout users prefer :)

@zerothi
Copy link
Owner

zerothi commented Nov 1, 2023

I really like the grayed out typing construct.
I agree the typing is horrendous, I thought it would help clarify how the arguments are allowed, but it is just too much, see here: https://zerothi.github.io/sisl/api/generated/sisl.Geometry.html#sisl.Geometry.auc2sc
I really would like it to just point to AtomsLike.

@pfebrer
Copy link
Contributor Author

pfebrer commented Nov 1, 2023

I really would like it to just point to AtomsLike.

See the trick here: pandas-dev/pandas#33025 (comment)

I think it would be very useful for AtomsLike and AtomsArgument.

I really like the grayed out typing construct.

This one: ?

image

@zerothi
Copy link
Owner

zerothi commented Nov 2, 2023

Great reference for hiding the information, lets do something like that. I'll play later.

As for the type hinting, I kind of prefer how it is shown in Geometry, no type-hints at the interface (for clarity), and an extensive type declaration in the Parameters section.
I am not fully sure why your PdosPlot shows differently from Geometry...

@pfebrer
Copy link
Contributor Author

pfebrer commented Nov 2, 2023

Don't you think the typehinted signature is clear enough if you give one line to each parameter? I like it because it gives you a quick idea of what the argument expects, as in the source code.

I think this:

Screenshot from 2023-11-01 18-23-45

Is more helpful than this:

Screenshot from 2023-11-02 09-58-41

(I know the typing is missing in the argument description)

@zerothi
Copy link
Owner

zerothi commented Nov 2, 2023

The typing should be present in the argument description, I don't mind having them in the interface, IFF they are simple lines, the Geometry example is really bad since there the typing gets excessively long. If it hinders readability, I don't want it ;)
Users don't really care, and if it becomes difficult to see the order of arguments, it becomes hard.
So in that case the 1-line per argument is great!

@pfebrer
Copy link
Contributor Author

pfebrer commented Nov 2, 2023

Yeah I agree, I would like that the typehints are shown in the signature with 1 line per argument only if it is possible to use the trick to avoid expanding the complex typehints.

I think for users that are already familiar with type hints that might help, and also it shows that the package is type hinted, which is not clear if the typehint appears only in the description.

@zerothi zerothi mentioned this issue Jan 23, 2024
10 tasks
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

No branches or pull requests

2 participants