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

add figpager as a third party package #18025

Closed
wants to merge 16 commits into from
Closed

add figpager as a third party package #18025

wants to merge 16 commits into from

Conversation

ebenp
Copy link
Contributor

@ebenp ebenp commented Jul 22, 2020

PR Summary

I wrote a class around matplotlib's single and multi page backends to control subplot layout and some figure elements such s text, lines and images to make single and multi page figure templates. This PR adds links to this template library to the third party documentation page in matplotlib's documentation.

PR Checklist

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

@story645
Copy link
Member

story645 commented Jul 22, 2020

please add Framework :: Matplotlib to the classifiers list in figpager's setup.py

@ebenp
Copy link
Contributor Author

ebenp commented Jul 22, 2020

Thanks! :) I've updated setup.py
ebenp/figpager@245ab1d

Copy link
Member

@jklymak jklymak left a comment

Choose a reason for hiding this comment

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

Thanks for adding this! You have a couple of formatting issues.

@@ -282,6 +282,14 @@ border, and logo.
.. image:: /_static/mpl_template_example.png
:height: 330px

figpager
=====
Copy link
Member

Choose a reason for hiding this comment

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

undescores must span the element above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I've extended them below.
ebenp@898e67f

figpager
=====

`figpager <https://pypi.org/project/figpager/>`_ provides customizable figure elements such as text, lines and images
Copy link
Member

Choose a reason for hiding this comment

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

Please wrap at 79 char....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I've wrapped it below.
ebenp@898e67f

Corrected heading and character wrap for figpager entry. Added period at sentence end.
Copy link
Member

@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

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

We try to keep image sizes small. Please run the image though a png compressor like https://tinypng.com/.

@ebenp
Copy link
Contributor Author

ebenp commented Jul 23, 2020

We try to keep image sizes small. Please run the image though a png compressor like https://tinypng.com/.

Thanks for thinking of the pixels! Thanks for linking to a compressor as well. I think I've updated things correctly.

Smaller image processed through tinypng and uploaded.
ebenp@f8d7f9a

Image path update in index.rst
ebenp@37a6fff#diff-688463b7d6617526f45f8caa3c0158c6

@tacaswell tacaswell added this to the v3.4.0 milestone Jul 23, 2020
Copy link
Member

@tacaswell tacaswell left a comment

Choose a reason for hiding this comment

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

👍 This should be squashed to 1 commit (either by @ebenp or via squash-merge).

@tacaswell
Copy link
Member

@ebenp you may also want to put in a JOSS (https://joss.theoj.org/) paper?

and subplot layout control for single or multi page output
`figpager <https://pypi.org/project/figpager/>`_ provides
customizable figure elements such as text, lines and images
and subplot layout control for single or multi page output.
Copy link
Member

Choose a reason for hiding this comment

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

and a subplot layout

anntzer and others added 4 commits July 23, 2020 11:32
This shaves off ~33% runtime from
```python
from pylab import *
import timeit

tx = figtext(.5, .5, "foo\nbar baz")
gcf().canvas.draw()
gcf().patch.set_visible(False)
r = gcf()._cachedRenderer
n, t = timeit.Timer(lambda: tx.draw(r)).autorange(); print(t / n, n, t)
```
because previously the calls to realpath were not cached and filesystem
access is slow.

(The font cache is likely already in trouble if fonts move in the
filesystem within the execution of a program given that there are other
layers of cache, so I wouldn't overly worry about that.  At best we may
want to expose a manual function to invalidate all caches if someone
really runs into that problem.)
@jklymak
Copy link
Member

jklymak commented Jul 23, 2020

All these commits should be squashed before merging....

QuLogic and others added 6 commits July 23, 2020 11:48
author ebenp <4080051+ebenp@users.noreply.github.com> 1595519414 -0400
committer ebenp <4080051+ebenp@users.noreply.github.com> 1595520159 -0400

Added fgpager info
@ebenp
Copy link
Contributor Author

ebenp commented Jul 23, 2020

Ugh. Tried to squash / rebase and utterly failed. I kept getting newer matplotlib changes interspersed. Anyway, I nuked and paved by re forking and updating the files. How do I reset the link to the respoitory? HALP. :)

ebenp@1b783f1

@jklymak
Copy link
Member

jklymak commented Jul 23, 2020

So sorry! I should have been more clear that you didn't need to do the squash if you weren't comfortable with the process.

@ebenp
Copy link
Contributor Author

ebenp commented Jul 23, 2020

So sorry! I should have been more clear that you didn't need to do the squash if you weren't comfortable with the process.

Oh, no worries. I thought I could figure it out. I think I was close but my changes kept editing other files; init.py and font_manager and I got worried. For the future I'll ask from help in squashing. I assume I pull and then squash? Or do I squash my commits and don't worry about it being out of sync with the current matplotlib?

@jklymak
Copy link
Member

jklymak commented Jul 23, 2020

I typically git rebase upstream/master, fix any conflicts, and then git log to see what the last commit on master is (say xxxx) and then do git rebase -i xxxx. https://matplotlib.org/3.3.0/devel/gitwash/index.html should go through this. If its not clear, let us know...

@ebenp
Copy link
Contributor Author

ebenp commented Jul 23, 2020

I typically git rebase upstream/master, fix any conflicts, and then git log to see what the last commit on master is (say xxxx) and then do git rebase -i xxxx. https://matplotlib.org/3.3.0/devel/gitwash/index.html should go through this. If its not clear, let us know...

Thanks! That is super helpful. For this draft pull request is everything ok despite me removing the repository? Should I recreate the draft pull request or create a PR?

@ebenp
Copy link
Contributor Author

ebenp commented Jul 23, 2020

@ebenp you may also want to put in a JOSS (https://joss.theoj.org/) paper?

Great idea! I'll have to come up with a spiffy logo!

@timhoffm
Copy link
Member

Generally, you don't need to worry about syncing. The GitHub UI will tell you if there is a merge conflict. And only then you need to sync (by rebasing on top of master).

@ebenp
Copy link
Contributor Author

ebenp commented Jul 23, 2020

Generally, you don't need to worry about syncing. The GitHub UI will tell you if there is a merge conflict. And only then you need to sync (by rebasing on top of master).

Ah ok. Yeah, I tried to be all fancy before I squashed through rebase and quickly was out of my depth. I won't worry about it in the future.

Thanks for the help and support all! This has been a really welcoming intro to matplotlib PRs.

@ebenp
Copy link
Contributor Author

ebenp commented Jul 24, 2020

@story645 Let me know if there's a way to relink my repository or if I should create a new PR.

@QuLogic
Copy link
Member

QuLogic commented Jul 24, 2020

I don't think there is a way. Probably be best to create a new PR if this one is no longer updating correctly.

@ebenp
Copy link
Contributor Author

ebenp commented Jul 25, 2020

Thanks! Sounds great. A new PR is here: #18059

@ebenp ebenp closed this Jul 25, 2020
@ebenp ebenp mentioned this pull request Jul 25, 2020
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants