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

Support for legend with "handles" and "labels" like MATLAB/matplotlib #260

Closed
liamtoney opened this issue Jan 2, 2019 · 18 comments · Fixed by #333
Closed

Support for legend with "handles" and "labels" like MATLAB/matplotlib #260

liamtoney opened this issue Jan 2, 2019 · 18 comments · Fixed by #333
Labels
feature request New feature wanted
Milestone

Comments

@liamtoney
Copy link
Member

Description of the desired feature

A wrapper for the GMT 6 legend command with support for specifying "handles" and "labels" as arguments, as in MATLAB (legend docs) and matplotlib (legend docs). The plotting commands (i.e., gmt.Figure.plot(), for now) are modified to return "handles" which are provided as inputs to gmt.Figure.legend(). This addresses part of #214 and #231.

Below is a minimal working example using my add-legend fork:

import gmt

fig = gmt.Figure()

h1 = fig.plot(x=[-5], y=[5], region=[-10, 10, -5, 10], projection='X3i/0', frame='a',
              style='a15p', pen='1.5p,purple')

h2 = fig.plot(x=[0], y=[5],
              style='t10p', color='cyan')

h3 = fig.plot(x=[5], y=[5],
              style='d5p', color='yellow', pen=True)

fig.legend([[h1, h2, h3], ['I am a star!', 'I am a triangle!', 'I am a diamond!']],
           F=True, D='g0/0+w2i+jCM')    

fig.show()

example

Are you willing to help implement and maintain this feature?

Yes. You can view the source code for my implementation here.

The good news:

  • The legend wrapper accepts either the standard specfile (see GMT legend docs) or a list containing a list of handles and a list of labels. So folks can still build a complex legend by creating a specfile.
  • The only modification to gmt.Figure.plot() is that it now returns kwargs. So existing users won't have to change their syntax for the plot command.

The bad news:

  • This implementation makes use of GMTTempFile since I don't know how to do it any other way (yet).
  • Right now I'm using regular expression matching (via re) to separate symbol type and size from the style argument of gmt.Figure.plot(), which is probably really sketchy.
  • This only works for symbol entries in the legend, i.e. entries in specfile which have the form:
    S - symbol size fill pen - text
    I feel like this covers a broad portion of typical legend use cases, though.
  • I have not written any tests.
@leouieda leouieda added the feature request New feature wanted label Jan 22, 2019
@leouieda
Copy link
Member

@liamtoney I'm waiting on some new features from GMT that might make this a lot easier (meaning that we might not have to implement it ourselves). But if it doesn't go through, we should definitely do this.

@liamtoney
Copy link
Member Author

Hi @leouieda, it seems like GMT 6 is pretty close to release. Just wanted to ping you for any updates on legend functionality being implemented in the coming release.

@leouieda
Copy link
Member

@liamtoney I kind of lost track of this in the main GMT repo (so many things happened in the mean time). I'll start a bit more heavy work on PyGMT soon now that we have 6.0.0rc2 packages for all platforms. I'll get back to this in a little while. Is that OK?

@liamtoney
Copy link
Member Author

@leouieda of course! Thanks for all of your hard work.

@weiji14 weiji14 added this to the 0.1.0 milestone Oct 4, 2019
@weiji14
Copy link
Member

weiji14 commented Oct 7, 2019

Hi @liamtoney, just wondering if you're still willing in implementing this and maybe submit a Pull Request. Quite keen on getting this legend function wrapped by the end of this month (October 2019) for the PyGMT workshop I'm hosting in November.

I can help out with writing some unit tests and bring the code up to date with GMT 6.0.0rc4 if you're short on time, but just thought I'd ask you first 😄 It'll tie in quite nicely with the colorbar wrapper at #332 I'm currently working on and close #231 once and for all.

@seisman
Copy link
Member

seisman commented Oct 7, 2019

FYI, @PaulWessel is implementing the similar feature on the GMT side. I think it also makes the pygmt legend easier to implement.

@weiji14
Copy link
Member

weiji14 commented Oct 7, 2019

Is that with the auto-legend feature at GenericMappingTools/gmt#1749?

@seisman
Copy link
Member

seisman commented Oct 7, 2019

Yes, I forgot to post the link to the PR.

@liamtoney
Copy link
Member Author

liamtoney commented Oct 7, 2019

@weiji14 I'm happy to help implement this. I don't have any experience writing tests, so I'd definitely appreciate help with that. Should we wait until the dust settles on GMT's auto-legend feature? It might change how I'd approach wrapping the command.

@weiji14
Copy link
Member

weiji14 commented Oct 7, 2019

I actually feel that we can go ahead with wrapping legend as it is now, and refactor our code to include the auto-legend stuff once that's merged in (it'll take time for a GMT6.0.0rc5 or 6.0.0 release to come out anyway).

@weiji14
Copy link
Member

weiji14 commented Oct 7, 2019

@weiji14 I'm happy to help implement this. I don't have any experience writing tests, so I'd definitely appreciate help with that. Should we wait until the dust settles on GMT's auto-legend feature? It might change how I'd approach wrapping the command.

Great! Start by creating a test_legend.py file under the pygmt/tests folder. You can have a look at my test_colorbar.py file for inspiration since it's similar. For example: this bit that just tests positioning of a colorbar:

"""
Tests colorbar
"""
import pytest
from .. import Figure
@pytest.mark.mpl_image_compare
def test_colorbar_using_paper_coordinates():
"""
Create colorbar positioned at 0cm,0cm with length 1cm and width 0.5cm.
"""
fig = Figure()
fig.colorbar(cmap="rainbow", position="x0c/0c+w1c/0.5c")
return fig

You'll want to replace colorbar with legend. Also if you're starting out, maybe try running everything outside of the python function and call fig.show() to display the plot.

Once you're happy with the plot, create a 'baseline' output plot using:

py.test --mpl-generate-path=baseline pygmt/tests/test_legend.py

After that, test your code by running make test. Let me know if you have any problems!

@PaulWessel
Copy link
Member

Thanks, I will try to wrap up the legend implementation on the C side in the next 1-2 days.

@shicks-seismo
Copy link

A legend function is something I've been looking for of late. I'm more than happy to help test any of the implementations mentioned above when they become available.

@liamtoney
Copy link
Member Author

liamtoney commented Oct 8, 2019

@weiji14 do I need to build rc4 from source to do dev stuff? I tried

conda install -c conda-forge/label/dev gmt=6.0.0rc4

into the pygmt environment defined in environment.yml and got package conflict errors. Would really like to avoid building from source if it's avoidable.


EDIT

I have 6.0.0rc2 on my machine , built from source a while ago. I did the hack

required_version = "6.0.0rc2"

in session.py and ran my MWE above with no trouble so I'll go from there for the time being.

@liamtoney
Copy link
Member Author

@weiji14 here's my try at a test for the legend. It passes. I'm not sure what sort of tests are good to write, so I'd appreciate some guidance on that. I also should re-visit the actual legend implementation.

https://github.com/liamtoney/pygmt/commit/cddf2cf18b5a12e2ec7fa1650f218b01267317a6

Let me know at what point it makes sense to make a PR out of this.

@weiji14
Copy link
Member

weiji14 commented Oct 8, 2019

The test looks a bit long, but it's a good start! We tend to want tests that test one specific 'unit' of functionality, for e.g. positioning of the legend, changing the colour of the border, etc. Also you might want to make sure that passing in a specfile works too.

You can actually submit a PR right now, and put 'WIP' in the title (call if `WIP Wrap legend', that'll make it easier for me or anyone else on the team to help you out by making 'suggested changes'.

@liamtoney liamtoney mentioned this issue Oct 8, 2019
5 tasks
@liamtoney
Copy link
Member Author

The test looks a bit long, but it's a good start! We tend to want tests that test one specific 'unit' of functionality, for e.g. positioning of the legend, changing the colour of the border, etc. Also you might want to make sure that passing in a specfile works too.

Gotcha on the unit test thing. I'll make some of those later this week if I have time.

You can actually submit a PR right now, and put 'WIP' in the title (call if `WIP Wrap legend', that'll make it easier for me or anyone else on the team to help you out by making 'suggested changes'.

Done. We can use that thread for further conversation if you want.

@weiji14
Copy link
Member

weiji14 commented Oct 8, 2019

P.S. If you'd like to understand testing a bit more, I highly recommend Automation Panda's Python Testing 101 series. In particular, the introduction and pytest one will be relevant for PyGMT. He's also got some videos if you prefer that style of learning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature wanted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants