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

[docs] expand documentation on 'group' for ranking task #3772

Merged
merged 8 commits into from
Jan 18, 2021
Merged

Conversation

jameslamb
Copy link
Collaborator

@jameslamb jameslamb commented Jan 16, 2021

Based on the discussion in #3708 (comment), this PR proposes adding more details to the explanation of group in the Python and R packages.

I think that the current description doesn't give enough details for someone unfamiliar with LightGBM to get started. I know that I personally really struggled to understand what "group" should look like the first time I did learning-to-rank with LightGBM.

@StrikerRUS
Copy link
Collaborator

Very nice addition!
Could you please add ... , see `Query Data <#query-data>`__ section below for more info (or something like that) here

// desc = **Note**: data should be grouped by query\_id

https://lightgbm.readthedocs.io/en/latest/Parameters.html#query-data

and unify that description at the bottom of the file with the proposed new one?

@jameslamb
Copy link
Collaborator Author

Very nice addition!
Could you please add ... , see `Query Data <#query-data>`__ section below for more info (or something like that) here

// desc = **Note**: data should be grouped by query\_id

https://lightgbm.readthedocs.io/en/latest/Parameters.html#query-data
and unify that description at the bottom of the file with the proposed new one?

thanks for pointing me to that! I tried an update in 6a42dff, let me know what you think.

I built the docs locally like this

python helpers/parameter_generator.py
cd docs
make clean html

And confirmed that the link to the "Query Data" section worked as expected.

Copy link
Collaborator

@StrikerRUS StrikerRUS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jameslamb Yeah, thanks for updating general params doc! Please check my comments.

R-package/R/lgb.Dataset.R Outdated Show resolved Hide resolved
include/LightGBM/config.h Outdated Show resolved Hide resolved
docs/Parameters.rst Outdated Show resolved Hide resolved
docs/Parameters.rst Outdated Show resolved Hide resolved
python-package/lightgbm/basic.py Outdated Show resolved Hide resolved
python-package/lightgbm/sklearn.py Outdated Show resolved Hide resolved
python-package/lightgbm/sklearn.py Outdated Show resolved Hide resolved
python-package/lightgbm/sklearn.py Outdated Show resolved Hide resolved
python-package/lightgbm/sklearn.py Outdated Show resolved Hide resolved
python-package/lightgbm/sklearn.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@StrikerRUS StrikerRUS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jameslamb Thanks! I believe it's nice addition for beginners.

Feel free to merge after fixing typos below and regenerating Rds.

Some R documentation files have changed. Please re-generate them and commit those changes.

R-package/R/lgb.Dataset.R Outdated Show resolved Hide resolved
R-package/R/lgb.Dataset.R Outdated Show resolved Hide resolved
python-package/lightgbm/basic.py Outdated Show resolved Hide resolved
python-package/lightgbm/basic.py Outdated Show resolved Hide resolved
python-package/lightgbm/basic.py Outdated Show resolved Hide resolved
python-package/lightgbm/sklearn.py Outdated Show resolved Hide resolved
python-package/lightgbm/sklearn.py Outdated Show resolved Hide resolved
python-package/lightgbm/sklearn.py Outdated Show resolved Hide resolved
python-package/lightgbm/sklearn.py Outdated Show resolved Hide resolved
python-package/lightgbm/sklearn.py Outdated Show resolved Hide resolved
jameslamb and others added 2 commits January 18, 2021 10:27
Co-authored-by: Nikita Titov <nekit94-08@mail.ru>
@jameslamb
Copy link
Collaborator Author

Thanks for catching my typos, sorry 😬

I just regenerated the R docs (ec63db8). Will merge this if / when Ci passes.

@jameslamb jameslamb merged commit 0e5eb9e into master Jan 18, 2021
@jameslamb jameslamb deleted the docs/jlamb branch January 18, 2021 17:58
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants