10000 [MRG] Updates on the scikit-learn sponsor page by cmarmo · Pull Request #15043 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

[MRG] Updates on the scikit-learn sponsor page #15043

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 25 commits into from
Oct 9, 2019
Merged

[MRG] Updates on the scikit-learn sponsor page #15043

merged 25 commits into from
Oct 9, 2019

Conversation

cmarmo
Copy link
Contributor
@cmarmo cmarmo commented Sep 20, 2019 •
edited by ogrisel
Loading

Reference Issues/PRs

Closes #14974

What does this implement/fix? Explain your changes.

Updates the funding section in the about us page.

TODO

Please, just comment about the content now: css will be fixed once the content well defined.

  • Separate active supporters from past supporters in two distinct sections
  • Decide whether we should keep or remove the logos on the home page:
    • If we keep them we should update the list: shall we expand 8000 the logos of all active supporters? If so, which order?
    • If we do not keep them we need to replace them by a link "scikit-learn development maintenance is financially supported by generous sponsors" or something similar
    • If we do not keep them we need to replace Google search with sphinx search
  • Improve the layout of the sponsor logos and text explanations in the funding section of about.html
  • Are there any institution / firm needs to be added to the list (past and present)?
  • Is the NumFocus Donate button still relevant? (will be updated in another PR if needed)

Thanks for your collaboration.

@jnothman
Copy link
Member

I like how this looks. Is it still WIP?

one area to improve: I think it's easy to currently misconstrue that the consortium is the primary/only funding source. So a preface might help clarify that several sources will be listed.

@cmarmo
Copy link
Contributor Author
cmarmo commented Sep 23, 2019

Thanks @jnothman for your answer.
I've added a small preface.
Yes, the PR is still WIP. I'm lacking some information.
I will add my questions directly in the code review.

cmarmo
cmarmo commented Sep 23, 2019
View reviewed changes
doc/about.rst Outdated
`The University of Sydney <https://sydney.edu.au/>`_ funds Joel Nothman since July 2017.
Andreas Müller received a grant to improve scikit-learn from the
`Alfred P. Sloan Foundation <https://sloan.org>`_ .
This grant supports the position of Nicolas Hug and Thomas J. Fan.
Copy link
Contributor Author
@cmarmo cmarmo Sep 23, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

@amueller, @NicolasHug, @thomasjpfan could you please check? Thanks!

cmarmo
cmarmo commented Sep 23, 2019
View reviewed changes
cmarmo
cmarmo commented Sep 23, 2019
View reviewed changes
cmarmo
cmarmo commented Sep 23, 2019
View reviewed changes
doc/about.rst

The 2011 International Granada sprint was made possible thanks to the support
of the `PSF <https://www.python.org/psf/>`_ and `tinyclues
<https://www.tinyclues.com/>`_.

Donating to the project
Copy link
Contributor Author

Choose a reason for hiding this comment

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

See also the discussion about NumFocus here #6603
@GaelVaroquaux and Co. (I don't know exactly who I need to ping...) is that still relevant? Who is taking care of that?
Thanks!

@cmarmo
Copy link
Contributor Author
cmarmo commented Sep 23, 2019 •
edited
Loading

Hello everybody: in 2ca0613 I have removed the logos on the homepage and changed the link text in the community section.
It looks consistent to me , but please, feel free to comment.

I have now a concern about the Google search engine (see also #7061), because without any other logos in the homepage its presence ends up in an unjustified Google visibility with respect to other funders.
So please tell me... :)
Either we re-add more logos (but then which one), either we switch to the sphinx search.
Thanks for your comments on that.

ogrisel
ogrisel reviewed Sep 23, 2019
View reviewed changes
Copy link
Member
@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

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

Some comments on the logo layout:

@jnothman
Copy link
Member
jnothman commented Sep 23, 2019 via email

thomasjpfan
thomasjpfan reviewed Sep 23, 2019
View reviewed changes
@cmarmo cmarmo changed the title [WIP] Updates on the scikit-learn sponsor page [MRG] Updates on the scikit-learn sponsor page Sep 24, 2019
@cmarmo
Copy link
Contributor Author
cmarmo commented Sep 24, 2019

Any reason this is still WIP?

Changed to [MRG] . Someone will answer my questions in the review phase... right? :)

@adrinjalali
Copy link
Member

@cmarmo I also had your intuition at first, but we apparently use WIP only when we're sure it's not in a good state and we're still working on it. You can remove the tag as soon as it's ready for review, and the review is in progress.

Writing this up, I realize how vague and undefined it is.

@jnothman
Copy link
Member
jnothman commented Sep 24, 2019 via email

@ogrisel
Copy link
Member
ogrisel commented Sep 25, 2019 •
edited
Loading

Summarizing some discussion we had yesterday IRL with @cmarmo @glemaitre and @jeremiedbb :

  • The bold labels "Platinum Members", "Gold Members"... for the Inria Foundation sponsors are still too prominent . I personally think we should get rid of those labels to declutter the layout and keep the highlight on the text explanations.

  • The layout "Logo followed by centered text explanation" that is used for most funding sources is reversed for "Inria Foundation" because we need many "grouped" logos for one funding line in this case. Instead one could experiment with getting rid of the sphinx markup constraints and use inline HTML + new entries in the theme.css so as to have text on the left, logos on the right for all funding sources, irrespective of the number of logos on the right hand side.

  • The Inria logo could be added to the list side-by-side with dataiku to keep the balance. The Inria support level is slightly different but let's keep things simple.

@ogrisel
Copy link
Member
ogrisel commented Sep 25, 2019 •
edited
Loading

I have refactored the description of the PR to turn it into a tick-able todo list. I think we should keep this PR as WIP as long as not all the boxes are ticked. Some of them just imply making a decision.

  • For the NumFocus button I think we can leave it as it is. If it needs to be updated in anyway it can be done in a dedicated PR.

  • For the home page, maybe we can try to expand the list of logos of all active supporters .

But then we have the problem of the order? Maybe oldest support to newest? Or shall we use a bit of javascript to shuffle the order on display?

@GaelVaroquaux
Copy link
Member
GaelVaroquaux commented Sep 25, 2019 via email

@cmarmo
Copy link
Contributor Author
cmarmo commented Oct 2, 2019

@ogrisel, I've added all the logos ... I'm not quite sure about the order ... let me know.

ogrisel
ogrisel approved these changes Oct 4, 2019
View reviewed changes
Copy link
Member
@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

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

LGTM, thanks very much @cmarmo!

@ogrisel
Copy link
Member
ogrisel commented Oct 4, 2019

Actually can you please add a link from the footer logos of the index.html page to the Funding section of the about.html page?

@cmarmo
Copy link
Contributor Author
cmarmo commented Oct 4, 2019

Actually can you please add a link from the footer logos of the index.html page to the Funding section of the about.html page?

Done: but I'm really unhappy with the text-decoration: underline popping up when mouse-over the link... and can't find where this property is defined, and no time this evening to do that ... maybe @thomasjpfan has some ideas? Thanks!

@ogrisel
Copy link
Member
ogrisel commented Oct 4, 2019

Maybe putting the <a/> inside the <div/> but around the <img/> tags?

@thomasjpfan
Copy link
Member

You can add a class="sk-landing-funding-link" to a and set:

a.sk-landing-funding-link:hover {
  text-decoration: none;
}

@cmarmo
Copy link
Contributor Author
cmarmo commented Oct 5, 2019

You can add a class="sk-landing-funding-link" to a and set:

a.sk-landing-funding-link:hover {
  text-decoration: none;
}

Right! Thanks! Done.

@cmarmo cmarmo mentioned this pull request Oct 7, 2019
Merged
@jnothman
Copy link
Member
jnothman commented Oct 8, 2019

I'm okay to get rid of the sponsors' logos on the homepage. Atm it it unclear that they are there because of sponsorship.

@jnothman
Copy link
Member
jnothman commented Oct 8, 2019

I like what's happening in about.html

@cmarmo
Copy link
Contributor Author
cmarmo commented Oct 8, 2019

I'm okay to get rid of the sponsors' logos on the homepage. Atm it it unclear that they are there because of sponsorship.

I agree with you... but I could not persuade @ogrisel ... :)

@ogrisel
Copy link
Member
ogrisel commented Oct 8, 2019

I'm okay to get rid of the sponsors' logos on the homepage. Atm it it unclear that they are there because of sponsorship.

I agree with you... but I could not persuade @ogrisel ... :)

Alright, let's remove them from the homepage then. I did not want to change how we acknowledge support unilaterally.

@GaelVaroquaux
Copy link
Member
GaelVaroquaux commented Oct 8, 2019 via email

@ogrisel
Copy link
Member
ogrisel commented Oct 8, 2019

Yesterday evening, I was thinking that it could get us in trouble with higher ups...

What is more trouble: having logos on the homepage or not having logos?

@GaelVaroquaux
Copy link
Member
GaelVaroquaux commented Oct 8, 2019 via email

@ogrisel
Copy link
Member
ogrisel commented Oct 8, 2019

Ok then we can re-add them with a short sentence such as:

"
[News box] [Community box] [Testimonials box]

Scikit-learn development and maintenance is supported by generous sponsors:
[logo 1] [logo 2] ... [logo k]
"

and we only keep the link to about.html#funding in the footer.

Previously this link was in the "Community" box of the homepage which was probably the source of confusion:

Screenshot from 2019-10-08 15-49-28

@amueller
Copy link
Member
amueller commented Oct 8, 2019

looks good :)

@ogrisel
Copy link
Member
ogrisel commented Oct 9, 2019

Thanks @cmarmo for bearing with the decision process. Here is a screenshot of the current state:

Screenshot from 2019-10-09 11-56-32

I am still +1 for merge :)

@ogrisel ogrisel merged commit 377462f into scikit-learn:master Oct 9, 2019
@ogrisel
Copy link
Member
ogrisel commented Oct 9, 2019

Thank you very much @cmarmo! We can always refine in further PRs if needed.

@GaelVaroquaux
Copy link
Member
GaelVaroquaux commented Oct 9, 2019 via email

@cmarmo cmarmo deleted the funders branch October 9, 2019 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jorisvandenbossche jorisvandenbossche jorisvandenbossche left review comments

@adrinjalali adrinjalali adrinjalali left review comments

@thomasjpfan thomasjpfan thomasjpfan left review comments

@jeremiedbb jeremiedbb jeremiedbb left review comments

@jnothman jnothman jnothman approved these changes

@ogrisel ogrisel ogrisel approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[RFC] Update of the 'Funding' page on scikit-learn.org
9 participants
@cmarmo @jnothman @adrinjalali @ogrisel @GaelVaroquaux @thomasjpfan @amueller @jorisvandenbossche @jeremiedbb
0