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

Fix some readme bits #16671

Merged
merged 4 commits into from
Mar 8, 2020
Merged

Fix some readme bits #16671

merged 4 commits into from
Mar 8, 2020

Conversation

QuLogic
Copy link
Member

@QuLogic QuLogic commented Mar 5, 2020

PR Summary

The README now contains some non-ASCII characters, so we should specify an encoding when reading it to produce the long description.

Also, fix the images when the description is put on PyPI; it doesn't have our whole repo, so a relative link doesn't work.

PR Checklist

  • [N/A] Has Pytest style unit tests
  • Code is Flake 8 compliant
  • [N/A] New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • [N/A] Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • [N/A] Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@QuLogic QuLogic added this to the v3.2.1 milestone Mar 5, 2020
@QuLogic QuLogic linked an issue Mar 5, 2020 that may be closed by this pull request
README.rst Outdated

.. image:: doc/_static/logo2.png
.. image:: https://matplotlib.org/_static/logo2.png
Copy link
Member

Choose a reason for hiding this comment

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

Why do you link this one to matplotlib.org, but the following one to github.com/matplotlib?

Copy link
Member

Choose a reason for hiding this comment

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

And, would it be possible to use the svg logo?

Copy link
Member Author

Choose a reason for hiding this comment

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

The latter was added directly to the repo, but wasn't in the docs site as I think it wasn't updated yet. Since it's now there, I'll link it instead because the shorter URL looks better in the unrendered file.

The image appears to be cached on PyPI's side, so let me see if I can find some docs on supported file types.

Copy link
Member Author

@QuLogic QuLogic Mar 7, 2020

Choose a reason for hiding this comment

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

I see SVG in the readme renderer for Warehouse, so I believe it should work and have updated the URL.

See the README on my fork. The SVG starts out bigger, so it's scaled to fit the readme width; not sure if we want to scale it back down to the previous image's size.

Copy link
Member

Choose a reason for hiding this comment

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

No strong opinion on the size, but just checked on mobile. If it‘s not prohibitively large on a Large screen desktop, leave it otherwise scale it down. The air motivation for SVG is that it links good on HiDPI screens.

QuLogic added 3 commits March 7, 2020 00:45
This otherwise will fail in non-UTF8 locales, and Python 3.6 (which
doesn't coerce back to UTF8.)

Fixes matplotlib#16670.
PyPI doesn't have our whole repo, so a relative link doesn't work.
@story645 story645 merged commit a3e4d0f into matplotlib:master Mar 8, 2020
@story645
Copy link
Member

story645 commented Mar 8, 2020

Thanks for fixing this, sorry about that!

@lumberbot-app
Copy link

lumberbot-app bot commented Mar 8, 2020

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

  1. Checkout backport branch and update it.
$ git checkout v3.2.x
$ git pull
  1. Cherry pick the first parent branch of the this PR on top of the older branch:
$ git cherry-pick -m1 a3e4d0f1befcabfcaeaf605ac4501cec2a229ce5
  1. You will likely have some merge/cherry-pick conflict here, fix them and commit:
$ git commit -am 'Backport PR #16671: Fix some readme bits'
  1. Push to a named branch :
git push YOURFORK v3.2.x:auto-backport-of-pr-16671-on-v3.2.x
  1. Create a PR against branch v3.2.x, I would have named this PR:

"Backport PR #16671 on branch v3.2.x"

And apply the correct labels and milestones.

Congratulation you did some good work ! Hopefully your backport PR will be tested by the continuous integration and merged soon!

If these instruction are inaccurate, feel free to suggest an improvement.

@QuLogic QuLogic deleted the readme-fixes branch March 8, 2020 00:54
story645 added a commit to story645/matplotlib that referenced this pull request Mar 8, 2020
story645 added a commit to story645/matplotlib that referenced this pull request Mar 9, 2020
QuLogic pushed a commit that referenced this pull request Mar 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants