Add alt text for gcc-tips #4175
Conversation
There was a problem hiding this comment.
Thank you so much for taking this on @akshitadixit ! This page has received very little attention in the 3+ years since this content was first introduced, and I understand that it's pretty difficult to interpret some of the plots there, which don't have any explanatory text. This is the first time I've actually read this page and even I was pretty confused by some of it.
I left some suggestions below, but we'll also need to wait for another maintainer to comment on my questions about possibly removing some of these plots.
By the way...I noticed that you made this PR from the master branch on your fork. Since you're planning to help us with other things (#4136 (comment)), in the future you'll probably find it useful to create a new branch on your fork for each pull request. That way, you can have multiple pull requests into this project active at the same time.
|
|
||
| .. image:: ./_static/images/gcc-bars.png | ||
| :align: center | ||
| :target: ./_static/images/gcc-bars.png | ||
| :alt: picture of a simple bar chart against running time |
There was a problem hiding this comment.
To be honest, I'm seeing this image for the first time and it doesn't make much sense to me. It seems to be saying that the runtime for training was lower as max_depth was increased. That doesn't make sense to me.
@StrikerRUS do you understand it?
this one: https://lightgbm.readthedocs.io/en/latest/_static/images/gcc-bars.png
|
|
||
| .. image:: ./_static/images/gcc-meetup-2.png | ||
| :align: center | ||
| :target: ./_static/images/gcc-meetup-2.png | ||
| :alt: comparison of cumulative speed versus the slowest density of each algorithm at various depths with v-2, O-3 remaining constant almost in all cases |
There was a problem hiding this comment.
I think these stacked area plots (https://lightgbm.readthedocs.io/en/latest/_static/images/gcc-meetup-2.png) are really difficult to understand, and that this one should just be removed.
@StrikerRUS what do you think?
|
|
akshitadixit
requested review from
chivee,
henry0312,
shiyu1994 and
wxchan
as code owners
Co-authored-by: James Lamb <jaylamb20@gmail.com>
Co-authored-by: James Lamb <jaylamb20@gmail.com>
| @@ -76,7 +76,7 @@ def __call__(self, preds, dataset): | |||
| elif argc == 3: | |||
| grad, hess = self.func(labels, preds, dataset.get_group()) | |||
| else: | |||
| raise TypeError("Self-defined objective function should have 2 or 3 arguments, got %d" % argc) | |||
| raise TypeError(f"Self-defined objective function should have 2 or 3 arguments, got {argc}") | |||
There was a problem hiding this comment.
|
What a relief to know I am not the only person who was confused after seeing the page. I almost gave up on the PR afraid to pull naive alt texts to such a project. I read your suggestions and realized what I misinterpreted. Thank you! |
Yes, sorry about that! I think that page could use some work. Thanks for pushing through it! Without your PR we probably would have not noticed that it needed some next explanatory text. |
|
ok @akshitadixit I have an update for you! I talked to @Laurae2 in another chat about this. Copying those comments below:
So given that, could you please change this pull request to remove all of the images EXCEPT for Sorry that you spent time on them and we're going to delete them but know that we wouldn't have made this change unless you brought it up! So this improvement of the docs is thanks to your efforts. |
|
Closing this to fix all of the mess I created 😅 |
|
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. |
Closes #4028
Added alt text for images in gcc-tips.rst, sorry for taking this long, was reading about things since I did not fully understand all of the images. Kindly review and suggest in case I missed on something.