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

Follow docstring conventions #106

Open
cbrnr opened this issue Sep 20, 2024 · 6 comments
Open

Follow docstring conventions #106

cbrnr opened this issue Sep 20, 2024 · 6 comments

Comments

@cbrnr
Copy link

cbrnr commented Sep 20, 2024

This issue is part of openjournals/joss-reviews#6641.

As @roualdes has already mentioned in #95, the docstring for predef_eeg seems to be broken. Since this is likely one of the first functions that new users will interact with, I think it is really important to get this docstring right. I'm not sure what's going on, but it seems like underscores are mangled with the following characters and interpreted as an underline.

Screenshot 2024-09-20 at 08 31 46

In addition, I strongly suggest to follow Julia standard docstring conventions. Currently, the structure differs quite a lot from what I'd expect. For example, I find it quite difficult to get to know the signature when the various kwargs are not listed in a single place. This is especially important because usually I'd like to see all the default values just to know which arguments are required.

If you compare this to the doc of e.g. LinearAlgebra.svd, I'd recommend to implement something that follows what they are doing (which seems to be the standard):

Screenshot 2024-09-20 at 08 36 59

@behinger
Copy link
Member

behinger commented Sep 20, 2024

hi!
Thanks for the suggestion; we will think about the style of docs - can you check the branch #joss-doc-suggestion ? We fixed quite a bit of docstrings already (e.g. your raised point about the _ should be fixed

Thanks for bearing with us on this :-) we are still learning a lot

@cbrnr
Copy link
Author

cbrnr commented Sep 20, 2024

Hm, I installed this branch, but the docstring doesn't look any different, not sure if I did it correctly?

(@v1.10) pkg> st
Status `~/.julia/environments/v1.10/Project.toml`
  [ed8ae6d2] UnfoldSim v0.3.2 `https://github.com/unfoldtoolbox/UnfoldSim.jl#joss-doc-suggestions`

I also wanted to add that my comment about default arguments applies to other functions/objects as well, e.g. SingleSubjectDesign does not mention which arguments are required and which are optional (and their defaults). I assume that this will also be the case for other functions in this package, and this makes it rather difficult to learn the API. What I would like to see, and I think this is also standard in Julia, is all arguments (and their defaults for optional args) at a glance.

@behinger
Copy link
Member

grafik
strange. Not sure what's going on?

Your point is well taken, and we will try to work on all docstrings but it will take a bit of time

@cbrnr
Copy link
Author

cbrnr commented Sep 20, 2024

strange. Not sure what's going on?

No idea either. I simply installed it with

]
add UnfoldSim#joss-doc-suggestions

This should be correct?

@behinger
Copy link
Member

Did you restart the REPL? Sometimes I have noticed it doesnt update the package - I just tried with ]activate -temp and installed as you mentioned.

can you doublecheck with pathof(UnfoldSim) that it points to UnfoldSim/lM1mg/src/UnfoldSim.jl this hash (iirc the hash should be the same regardless of julia version)?

@cbrnr
Copy link
Author

cbrnr commented Sep 20, 2024

Did you restart the REPL? Sometimes I have noticed it doesnt update the package - I just tried with ]activate -temp and installed as you mentioned.

Ah yes, restarting the REPL fixed it! Thanks! The comment on following established standards (specifically with respect to arguments) still stands though.

@cbrnr cbrnr changed the title Docstring for predef_eeg seems to be broken Follow docstring conventions Sep 27, 2024
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