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

Optional args style #2547

Merged
merged 2 commits into from
Nov 17, 2020
Merged

Optional args style #2547

merged 2 commits into from
Nov 17, 2020

Conversation

kescobo
Copy link
Contributor

@kescobo kescobo commented Nov 16, 2020

This is the second of 3 PRs replacing #2488 now that the big reorg has happened. These should not change or add any functionality, but are instead intended to unify some style conventions.

Here, I deal with optional arguments in docstrings. There isn't a strong convention here - I looked through Base docstrings, and their style is inconsistent as well. One thing that does seem consistent is that multiple optional arguments have nested brackets, eg [, opt1[, opt2] ]. There aren't many examples in this package, but the principle I went with is that everything that is optional (including separators and spaces) should be inside brackets. So eg

foo(required[, opt])
bar([opt, ]required)

For multiple optional args, I did the same thing, with nested brackets. One thing that may or may not be controversial is that I put spaces between closing brackets.

baz(required[, opt1[, opt2] ])

There are examples and counterexamples of all of these things in Base, and I could not find a style guide that specifies (here's the section in blue style as example).

@bkamins
Copy link
Member

bkamins commented Nov 16, 2020

Looks good. Can you please check if https://github.com/JuliaData/DataFrames.jl/blob/master/CONTRIBUTING.md has an appropriate rule mentioned (so that we collect the style recommendations there).

@kescobo
Copy link
Contributor Author

kescobo commented Nov 16, 2020

Can you please check if https://github.com/JuliaData/DataFrames.jl/blob/master/CONTRIBUTING.md has an appropriate rule mentioned (so that we collect the style recommendations there).

I was about to push the 3rd one. Rule about spaces is already there, can I include this and the one about zeros in floats in the same PR to avoid test churn? (it will also avoid merge conflict)

@kescobo
Copy link
Contributor Author

kescobo commented Nov 16, 2020

Might also be useful to settle on language before pushing. I was thinking

* In docstrings, optional arguments, including separators and spaces, are surrounded by brackets, 
  e.g. `mymethod(required[, optional1[, optional2] ]; kwargs...)`

@bkamins
Copy link
Member

bkamins commented Nov 16, 2020

can I include this and the one about zeros in floats in the same PR to avoid test churn? (it will also avoid merge conflict)

OK

Might also be useful to settle on language before pushing

Looks good to me

@kescobo kescobo mentioned this pull request Nov 16, 2020
@bkamins bkamins added the non-breaking The proposed change is not breaking label Nov 16, 2020
@bkamins bkamins added this to the 1.x milestone Nov 16, 2020
Co-authored-by: Milan Bouchet-Valat <[email protected]>
@bkamins bkamins merged commit d938f9b into JuliaData:master Nov 17, 2020
@bkamins
Copy link
Member

bkamins commented Nov 17, 2020

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
non-breaking The proposed change is not breaking
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants