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

[R-package] Add R logo (fixes #3331, #3332) #3336

Merged
merged 8 commits into from
Aug 30, 2020
Merged

[R-package] Add R logo (fixes #3331, #3332) #3336

merged 8 commits into from
Aug 30, 2020

Conversation

jameslamb
Copy link
Collaborator

@jameslamb jameslamb commented Aug 27, 2020

This PR introduces a hex logo for the R package! (#3331) These logos are very popular in the R community (see, for example, https://github.com/rstudio/hex-stickers). Thanks to @guolinke for generating these logos.

It adds this file in man/figures/logo.png, the preferred place recognized by {pkgdown}.

Following @StrikerRUS 's suggestion in #3327 (comment), I also generated favicons for the site using pkgdown::build_favicons() (#3332).

{pkgdown} site looks good (if you squint you can see the favicon in the tab)

image

and this renders nicely in GitHub too!

image

@jameslamb jameslamb added the doc label Aug 27, 2020
@jameslamb jameslamb requested a review from Laurae2 as a code owner August 27, 2020 06:29
@jameslamb
Copy link
Collaborator Author

@StrikerRUS I created this as a LightGBM branch so we can test on readthedocs. Could you enable the build there?

@StrikerRUS
Copy link
Collaborator

@jameslamb

Could you enable the build there?

Sure! Just enabled.
https://readthedocs.org/projects/lightgbm/builds/

@jameslamb
Copy link
Collaborator Author

* checking for future file timestamps ... NOTE
unable to verify current time

huh, never saw that before. Looking into it. Since that problem doesn't seem isolated to a particular operating system, R version, or build type, I would be surprised to learn that it's related to the changes in #3335

@StrikerRUS
Copy link
Collaborator

I think we should add LightGBM_logo_hex.svg into https://github.com/microsoft/LightGBM/tree/master/docs/logo to be consistent with our FAQ-15: You can find LightGBM's logo in different file formats and resolutions here. if one want to add hex logo.
@jameslamb Is it possible to copy file from ../docs/logo to the R-package target location at a build time or provide customized path with the aim to not duplicate files?

@jameslamb
Copy link
Collaborator Author

I think we should add LightGBM_logo_hex.svg into https://github.com/microsoft/LightGBM/tree/master/docs/logo to be consistent with our FAQ-15: You can find LightGBM's logo in different file formats and resolutions here. if one want to add hex logo.
@jameslamb Is it possible to copy file from ../docs/logo to the R-package target location at a build time or provide customized path with the aim to not duplicate files?

I did add the SVG there, but for the {pkgdown} stuff to work, I believe your logo file has to just be called logo.png and it has to be in man/figures. When I tried with other locations, it wasn't found when rendering the {pkgdown} site. I think it's ok for the PNG to be in the expected place in the R package (to make {pkgdown} work) and for the SVG to be in docs/logo.

@jameslamb
Copy link
Collaborator Author

* checking for future file timestamps ... NOTE
unable to verify current time

huh, never saw that before. Looking into it. Since that problem doesn't seem isolated to a particular operating system, R version, or build type, I would be surprised to learn that it's related to the changes in #3335

This was just reported on the r-pkg-devel mailing list. Apparently R CMD check --as-cran has a dependency on two small "what time is it right now" services. One of which doesn't work with the way R is asking it for the time, and the other of which is currently down 🙃

https://stat.ethz.ch/pipermail/r-package-devel/2020q3/005928.html

@jameslamb
Copy link
Collaborator Author

I'm able to reproduce the time problem on my laptop too. If it is still a problem a few hours from now, I'll just bump the number of allowed checks so our CI can passs.

We shouldn't stop all development on LightGBM just because R doesn't know what time it is 😂

@StrikerRUS
Copy link
Collaborator

@jameslamb

I did add the SVG there, ...

OK, I see. Seems there is no room for logo path customization. But at least, according to this, logo can be in SVG:
https://github.com/r-lib/pkgdown/blob/3b4d5b3d724d923973cc28f1c975833c8dba0f29/R/build-logo.R#L15

@jameslamb
Copy link
Collaborator Author

OK, I see. Seems there is no room for logo path customization. But at least, according to this, logo can be in SVG:
https://github.com/r-lib/pkgdown/blob/3b4d5b3d724d923973cc28f1c975833c8dba0f29/R/build-logo.R#L15

yep! It can be but it doesn't have to be. I've generated the favicons once from R-package/man/figures/logo.png. Are you saying I should re-do it with the SVG?

@StrikerRUS
Copy link
Collaborator

@jameslamb

Are you saying I should re-do it with the SVG?

Ah, no, I think there is no difference from which source those tiny images were generated. I meant place svg in man R-package/man/figures/logo.svg instead of png to have good scaling.

@jameslamb
Copy link
Collaborator Author

oh I see! Ok I can do that right now

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.

Amazing, thank you!

Seems that pkgdown is happy too!

image

image

@jameslamb
Copy link
Collaborator Author

WOO awesome!

@jameslamb jameslamb merged commit 7e11d4a into master Aug 30, 2020
@jameslamb jameslamb deleted the feat/hex-logo branch August 30, 2020 04:14
@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.

2 participants