issue_comments

Table actions

8 rows where issue = 200593854 sorted by updated_at descending

✎ View and edit SQL

This data as json, CSV (advanced)

Suggested facets: reactions, created_at (date), updated_at (date)

author_association 3

issue 1

  • transfer projection to implied subplots when faceting · 8 ✖
id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
501857505 https://github.com/pydata/xarray/pull/1205#issuecomment-501857505 https://api.github.com/repos/pydata/xarray/issues/1205 MDEyOklzc3VlQ29tbWVudDUwMTg1NzUwNQ== stale[bot] 26384082 2019-06-13T20:00:57Z 2019-06-13T20:00:57Z NONE

In order to maintain a list of currently relevant issues, we mark issues as stale after a period of inactivity

If this issue remains relevant, please comment here or remove the stale label; otherwise it will be marked as closed automatically

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   transfer projection to implied subplots when faceting 200593854
315179563 https://github.com/pydata/xarray/pull/1205#issuecomment-315179563 https://api.github.com/repos/pydata/xarray/issues/1205 MDEyOklzc3VlQ29tbWVudDMxNTE3OTU2Mw== jhamman 2443309 2017-07-13T19:34:04Z 2017-07-13T19:34:04Z MEMBER

@vnoel and @fmaussion - my apologies for being quite tardy in looking at this PR. I think this is a valuable change but I do have one comment on the implementation.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   transfer projection to implied subplots when faceting 200593854
272612691 https://github.com/pydata/xarray/pull/1205#issuecomment-272612691 https://api.github.com/repos/pydata/xarray/issues/1205 MDEyOklzc3VlQ29tbWVudDI3MjYxMjY5MQ== fmaussion 10050469 2017-01-14T09:22:48Z 2017-01-14T09:22:48Z MEMBER

I know at least one usage where the projection keyword is needed for something else than cartopy (for polar plots, see example here by @hoonhout), but I don't know how often do people need a transform though.

In the end, I'm still in favor of this change: - I suspect the large majority of the use cases are for cartopy - it's still possible to override the this default behavior by setting the projection kwarg yourself

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   transfer projection to implied subplots when faceting 200593854
272416983 https://github.com/pydata/xarray/pull/1205#issuecomment-272416983 https://api.github.com/repos/pydata/xarray/issues/1205 MDEyOklzc3VlQ29tbWVudDI3MjQxNjk4Mw== fmaussion 10050469 2017-01-13T10:59:53Z 2017-01-13T10:59:53Z MEMBER

As it is, the xarray code becomes tied up with cartopy. If cartopy somehow changes its syntax (e.g. by renaming the transform kw) this code won't work.

cartopy uses transform because this is an interface provided by matlotlib: http://matplotlib.org/users/transforms_tutorial.html . This is not likely to change anytime soon.

 
{
    "total_count": 1,
    "+1": 1,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   transfer projection to implied subplots when faceting 200593854
272415578 https://github.com/pydata/xarray/pull/1205#issuecomment-272415578 https://api.github.com/repos/pydata/xarray/issues/1205 MDEyOklzc3VlQ29tbWVudDI3MjQxNTU3OA== vnoel 731499 2017-01-13T10:52:32Z 2017-01-13T10:52:32Z CONTRIBUTOR

As it is, the xarray code becomes tied up with cartopy. If cartopy somehow changes its syntax (e.g. by renaming the transform kw) this code won't work.

Maybe there's another, more abstract way to do this?

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   transfer projection to implied subplots when faceting 200593854
272414825 https://github.com/pydata/xarray/pull/1205#issuecomment-272414825 https://api.github.com/repos/pydata/xarray/issues/1205 MDEyOklzc3VlQ29tbWVudDI3MjQxNDgyNQ== fmaussion 10050469 2017-01-13T10:48:49Z 2017-01-13T10:48:49Z MEMBER

In that case, matplotlib will get an unintended projection argument. That is a corner case though.

Yes, I was about to ask @jhamman and @shoyer about their opinion about this. I'm actually not sure if this really is a corner case...

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   transfer projection to implied subplots when faceting 200593854
272414593 https://github.com/pydata/xarray/pull/1205#issuecomment-272414593 https://api.github.com/repos/pydata/xarray/issues/1205 MDEyOklzc3VlQ29tbWVudDI3MjQxNDU5Mw== vnoel 731499 2017-01-13T10:47:28Z 2017-01-13T10:47:28Z CONTRIBUTOR

One concern I have is that it is conceivable that one might pass a transform keyword to plot() intended to interact with another library that hijacks matplotlib (i.e. not cartopy). In that case, matplotlib will get an unintended projection argument. That is a corner case though.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   transfer projection to implied subplots when faceting 200593854
272412800 https://github.com/pydata/xarray/pull/1205#issuecomment-272412800 https://api.github.com/repos/pydata/xarray/issues/1205 MDEyOklzc3VlQ29tbWVudDI3MjQxMjgwMA== fmaussion 10050469 2017-01-13T10:37:57Z 2017-01-13T10:37:57Z MEMBER

Thanks! I think we need to document this somewhere, and also some tests would be nice.

Do you feel like trying to add the same functionality to normal 2d plots also? (this will represent a little more work so if you don't have time no big deal).

Also, you should credit yourself in the "what's new" section of whats-new.rst

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   transfer projection to implied subplots when faceting 200593854

Advanced export

JSON shape: default, array, newline-delimited, object

CSV options: 

CREATE TABLE [issue_comments] (
   [html_url] TEXT,
   [issue_url] TEXT,
   [id] INTEGER PRIMARY KEY,
   [node_id] TEXT,
   [user] INTEGER REFERENCES [users]([id]),
   [created_at] TEXT,
   [updated_at] TEXT,
   [author_association] TEXT,
   [body] TEXT,
   [reactions] TEXT,
   [performed_via_github_app] TEXT,
   [issue] INTEGER REFERENCES [issues]([id])
);
CREATE INDEX [idx_issue_comments_issue]
    ON [issue_comments] ([issue]);
CREATE INDEX [idx_issue_comments_user]
    ON [issue_comments] ([user]);