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

ds/concat* arglist and docstring improvements #412

Open
harold opened this issue Aug 20, 2024 · 2 comments
Open

ds/concat* arglist and docstring improvements #412

harold opened this issue Aug 20, 2024 · 2 comments
Labels
good first issue Good for newcomers

Comments

@harold
Copy link
Contributor

harold commented Aug 20, 2024

Autocomplete in my editor helpfully offers these suggestions:
image

The docstring for ds/concat says this:

Concatenate datasets in place using a copying-concatenation.

Which I find sort of confusing given that the three functions concat, concat-inplace, and concat-copying seem to imply that in place and copying are different.

Improve arglists

Also, the arglist of all three functions is: [dataset & args], which sort of obscures the fact that you're supposed to pass in many arguments each a dataset. Would [& datasets] be better? ... clojure.core/concat has [x y & zs] which I also sort of don't like because it obscures the fact that the arguments are supposed to be sequences.

Improve docstrings

My proposal would be for re-written docstrings on all three of these functions with

  1. mention of the input and output return types
  2. a short working example of usage, so users don't have to wonder if they need to call apply
  3. an indication in all three to which one is the one you probably want (is it ds/concat?) with a short explanation of why you occasionally might want the other two, with, if possible, a short example of their ideal use...
@cnuernber
Copy link
Collaborator

I did the absolute minimum possible to make the docstrings clearer - so this is partially addressed at this point - feel free to repoen if you want to have someone tackle this is more depth.

@harold harold added the good first issue Good for newcomers label Sep 12, 2024
@harold
Copy link
Contributor Author

harold commented Sep 12, 2024

bd8dacb

Agreed that's an improvement, I still think examples would go a long way. I'll put the good first issue tag on here and some enthusiast will come and do it. (:

@harold harold reopened this Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants