home / github / issue_comments

Menu
  • Search all tables
  • GraphQL API

issue_comments: 871121868

This data as json

html_url issue_url id node_id user created_at updated_at author_association body reactions performed_via_github_app issue
https://github.com/pydata/xarray/pull/4909#issuecomment-871121868 https://api.github.com/repos/pydata/xarray/issues/4909 871121868 MDEyOklzc3VlQ29tbWVudDg3MTEyMTg2OA== 14371165 2021-06-30T06:05:54Z 2021-06-30T06:05:54Z MEMBER

It would be nice to factor out some of the helper functions into plot.utils or elsewhere, and especially to use the @_plot2d decorator, but there is not need to squeeze that into this PR.

Thanks, @TomNicholas. My original plan and attempts was to use _plot2d. But because the return type originally was so different to the other plot functions (list of pathcollections vs pathcollections) it was impossible to use it without pretty much if-casing the entire _plot2d. I also feel _plot2d makes it pretty hard to read and understand the code.

I think it might be doable now that the function only returns pathcollection. I'll take a stab at that again in a different PR.

If you're worried about this not being fully robust until other plotting methods are condensed into one code path then I would just suggest not making it callable as a plot method yet (or renaming to _scatter as you suggested). But I also think it's fine to release as is.

Ok, I'll rename it to a private name. But I think it's good that it's still callable, so that others can easily try it out without any promises that the API will stay the same. I was also considering changing/removing some variable names related to legend compared to the dataset version, so it's good to let that it be private until that's been figured out.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  807939559
Powered by Datasette · Queries took 0.95ms · About: xarray-datasette