search

GenericMappingTools / pygmt

A Python interface for the Generic Mapping Tools.
https://www.pygmt.org
BSD 3-Clause "New" or "Revised" License
758 stars 220 forks source link

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

Closed liamtoney closed 5 years ago

liamtoney commented 5 years ago

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 bad news:

leouieda commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

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

weiji14 commented 5 years ago

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 :smile: 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 commented 5 years ago

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

weiji14 commented 5 years ago

Is that with the auto-legend feature at https://github.com/GenericMappingTools/gmt/pull/1749?

seisman commented 5 years ago

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

liamtoney commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

@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:

https://github.com/GenericMappingTools/pygmt/blob/cbca922bcc2440cb9506c7de24a0cec3b672a527/pygmt/tests/test_colorbar.py#L1-L16

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 commented 5 years ago

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

shicks-seismo commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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.