Skip to content

Conversation

clarkfitzg
Copy link
Member

Opening this to provide visibility as I work on this PR.

Currently this has x and y string arguments for 2d plot working, but everything else still needs lots of work. I'll let people know when it's ready for review.

@clarkfitzg
Copy link
Member Author

@shoyer Here's what we currently have:

In [15]: g = xray.plot.FacetGrid(t, col='time', col_wrap=2)

In [16]: g.map_dataarray(xray.plot.contourf, 'lon', 'lat')
Out[16]: <xray.plot.facetgrid.FacetGrid at 0x10de082d0>

image

Right now this assumes that the function for map_dataarray has the same signature as 2d xray plotting methods.

@clarkfitzg
Copy link
Member Author

Here's what a 4d array looks like.

In [26]: t4d = xray.concat([t2, t2 + 50], pd.Index(['normal', 'hot'], name='fourth_dim'))

# This is a 4d array
In [27]: t4d.coords
Out[27]: 
Coordinates:
  * lat         (lat) float32 75.0 72.5 70.0 67.5 65.0 62.5 60.0 57.5 55.0 ...
  * lon         (lon) float32 200.0 202.5 205.0 207.5 210.0 212.5 215.0 ...
  * time        (time) datetime64[ns] 2013-01-01 2013-03-04T12:00:00
  * fourth_dim  (fourth_dim) object 'normal' 'hot'

In [28]: g = xray.plot.FacetGrid(t4d, col='time', row='fourth_dim')

In [29]: g.map_dataarray(xray.plot.imshow, 'lon', 'lat')
Out[29]: <xray.plot.facetgrid.FacetGrid at 0x10f5aad90>

image

@jhamman
Copy link
Member

jhamman commented Aug 14, 2015

@clarkfitzg - These are looking really great. I think this is going to be a really useful tool.

The colorbar label is too close to the colorbar tick label, are we manually placing that or is this a matplotlib thing?

@clarkfitzg
Copy link
Member Author

Thanks! The colorbar label spacing is a matplotlib thing- it only became a problem after rotating it to match the row labels. So I expect that I can fix it by changing the anchor point or placing it manually.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe you can write something like the facet_data method from Seaborn's FacetGrid? That seems like a nice separation of the iteration logic from the plotting logic

Copy link
Member Author

Choose a reason for hiding this comment

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

That's on my todo list- I better post that list here.

@clarkfitzg
Copy link
Member Author

For this PR:

  • facet on rows and columns
  • Align label on colorbar
  • Label figure axes
  • Use xray plotting logic to make nice colorbar
  • Make and test choices around colorbar

EDIT - going to revisit plt.contour(color='k') after #537 gets worked out.

  • separate iteration over data from plotting - edge case: require the index to actually be unique .is_unique.
  • Use core string formatting for titles
  • tests for all of above
    • test col_wrap
  • write and organize docs
    • docs for public attribute- axes and such

On this PR or possibly a different one:

  • Convenience method to easily make faceted plots- something like darray.plot(col='time')
  • port Seaborn's aspect capabilities
  • matplotlib quiver plot for map example
  • Create new xray.tutorial module with function like load_example_dataset for plotting examples

Different PR:

  • Facets and maps
  • Inline documentation and plots for API reference

@clarkfitzg clarkfitzg mentioned this pull request Aug 17, 2015
Copy link
Member

Choose a reason for hiding this comment

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

This is a minor point, but take a look at the PEP8 indentation guidelines:
https://www.python.org/dev/peps/pep-0008/#indentation

This is an example of the first "No" case :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Just ran this through pep8 command line utility- lots of violations. Fixed.

@shoyer
Copy link
Member

shoyer commented Sep 4, 2015

For the simple example in the doc -- use imshow instead of contourf? The result is definitely more aesthetically pleasing:
image

Copy link
Member

Choose a reason for hiding this comment

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

My inclination is to not add this convenience shortcut, for two reasons:

  1. It's a departure from Seaborn, so it's something new for users to learn.
  2. It's not really necessary -- iterating over grid.axes.flat is much more explicit (generally a good thing), and only slightly more verbose.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added this with the idea of using an abc with this class: https://docs.python.org/2/library/collections.html#collections-abstract-base-classes.

Reviewing the available abc's, I don't think we stand to gain any benefit from them. At the same time, it is conceptually a 2d array. I suppose that structure is captured well enough in the axes and name_dicts.

Copy link
Member

Choose a reason for hiding this comment

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

Let's call this parameter data, like Seaborn. Eventually we might support dataset objects, too.

@clarkfitzg
Copy link
Member Author

@shoyer Latest commit also added a little more to the docs on how to use the attributes data axes, and name_dicts.

Copy link
Member

Choose a reason for hiding this comment

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

These variables are all duplicated between private versions (below) and public versions (above). For now, let's stick with the conservative choice of keeping them private like seaborn?

Copy link
Member Author

Choose a reason for hiding this comment

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

good catch

doc/plotting.rst Outdated
Copy link
Member Author

Choose a reason for hiding this comment

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

what is this doing? I think nothing.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe this is undoing the seaborn import? If it doesn't seem to be changing the doc build, take it out.

doc/api.rst Outdated
Copy link
Member

Choose a reason for hiding this comment

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

one more suggestion: clarkfitzg#2

we can remove this methods and let them just show up on the doc page for FacetGrid.

@shoyer
Copy link
Member

shoyer commented Sep 4, 2015

I think this is pretty much good to go. Merge at your discretion!

don't show FacetGrid methods directly in API docs
@clarkfitzg
Copy link
Member Author

Ok, going to merge once the build passes. Will continue with the rest of the stuff next week. @shoyer should I rebase or squash these commits?

@shoyer
Copy link
Member

shoyer commented Sep 4, 2015

I haven't been very careful about insisting on a clean git history so far, so don't worry about rebase/squash.

clarkfitzg added a commit that referenced this pull request Sep 4, 2015
@clarkfitzg clarkfitzg merged commit f26b29a into pydata:master Sep 4, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants