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

Improve sort docs #2931

Closed
bkamins opened this issue Nov 5, 2021 · 7 comments · Fixed by #2941
Closed

Improve sort docs #2931

bkamins opened this issue Nov 5, 2021 · 7 comments · Fixed by #2941
Labels
Milestone

Comments

@bkamins
Copy link
Member

bkamins commented Nov 5, 2021

Current docs for sort related functions is very brief. it should be improved.

@bkamins bkamins added the doc label Nov 5, 2021
@bkamins bkamins added this to the 1.3 milestone Nov 5, 2021
@bkamins bkamins modified the milestones: 1.3, 1.4 Nov 14, 2021
@Chandu-4444
Copy link
Contributor

I'd like to improve the doc, can you please help on where do I get started?

@bkamins
Copy link
Member Author

bkamins commented Nov 17, 2021

Thank you!

Do you need help with explaining the details how things work or you know the details but would only like help how to write the docstring and doc?

In general we need to improve:

  • docstrings for sort, sort!, issorted, sortperm
  • manual in section describing how sorting works

Biggest things that are missing:

  • no explanation how sorting on multiple columns works (it does lexicographic sort)
  • no explanation how extra arguments are handled (e.g. in docstring of sort we write See sort! for a description of other keyword arguments. and in the docstring of sort! we do not provide any such explanations)

@Chandu-4444
Copy link
Contributor

By looking at the docs and the docstrings, what I've understood are the following points:

  • There's no explanation about the other keyword arguments.
  • Yeah, sorting on multiple columns is done lexicographically (And it is something that should be mentioned for someone new)

I'll try improving those, and I need one clarification: Is it sufficient to add explanation about the other keyword arguments in the docstrings for sort! itself, or is it need to be done in other docstrings too (sort, sortperm)?

Also, one thing and this help would be highly appreciated: How do I work with this repo locally (Or any other Julia package repo)? How do I use this source instead of DataFrames packages that I installed through Pkg?

Thanks!

@bkamins
Copy link
Member Author

bkamins commented Nov 18, 2021

I think it is sufficient to explain the kwargs in sort!.

How do I work with this repo locally (Or any other Julia package repo)? How do I use this source instead of DataFrames packages that I installed through Pkg?

I am here describing you a full workflow given your permission level status:

  1. Fork DataFrames.jl on GitHub (so that you have your own fork to which you can push commits on GitHub)
  2. Create a project environment on your local computer, if you do not know how to do it it is described here: https://pkgdocs.julialang.org/dev/environments/
  3. In this environment dev DataFrames.jl package, which is explained here: https://pkgdocs.julialang.org/dev/managing-packages/#developing
  4. To the dev folder that gets created clone YOUR fork (it is important)
  5. Make the changes in the files in the dev folder (you will be able to test them, if you activate the environment you created in step 3. and do using DataFrames your fork will be loaded by Julia)
  6. Commit the changes in git to your local environment
  7. Push the changes. Now your fork on GitHub will have the changes
  8. Now GitHub will ask you if you want to make a PR to the central repository (this repository we are having the discussion right now)

(side note: there are easier ways to do this if you want to make smaller changes, but I give you a full workflow that will work in all scenarios no matter how complex)

@Chandu-4444
Copy link
Contributor

I think it is sufficient to explain the kwargs in sort!.

How do I work with this repo locally (Or any other Julia package repo)? How do I use this source instead of DataFrames packages that I installed through Pkg?

I am here describing you a full workflow given your permission level status:

  1. Fork DataFrames.jl on GitHub (so that you have your own fork to which you can push commits on GitHub)
  2. Create a project environment on your local computer, if you do not know how to do it it is described here: https://pkgdocs.julialang.org/dev/environments/
  3. In this environment dev DataFrames.jl package, which is explained here: https://pkgdocs.julialang.org/dev/managing-packages/#developing
  4. To the dev folder that gets created clone YOUR fork (it is important)
  5. Make the changes in the files in the dev folder (you will be able to test them, if you activate the environment you created in step 3. and do using DataFrames your fork will be loaded by Julia)
  6. Commit the changes in git to your local environment
  7. Push the changes. Now your fork on GitHub will have the changes
  8. Now GitHub will ask you if you want to make a PR to the central repository (this repository we are having the discussion right now)

(side note: there are easier ways to do this if you want to make smaller changes, but I give you a full workflow that will work in all scenarios no matter how complex)

Thank you! That was really informative. I'll start right away..

@Chandu-4444
Copy link
Contributor

I was trying to add a few examples to issorted. Do I need to hard code all the examples by copy-pasting them from my terminal to the docstrings? Or is there another way?

@bkamins
Copy link
Member Author

bkamins commented Nov 19, 2021

copy-pasting them from my terminal

I always do it this way as it is fastest and easiest

Or is there another way?

There is, https://juliadocs.github.io/Documenter.jl/stable/man/doctests/#Fixing-Outdated-Doctests, but I use it only if I make larger changes in the package.

@bkamins bkamins linked a pull request Nov 19, 2021 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants