issue_comments

Table actions

16 rows where issue = 748379763 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

  • plot tests: ensure all figures are closed · 16 ✖
id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
734802557 https://github.com/pydata/xarray/pull/4600#issuecomment-734802557 https://api.github.com/repos/pydata/xarray/issues/4600 MDEyOklzc3VlQ29tbWVudDczNDgwMjU1Nw== yashsaboo 24938400 2020-11-27T12:03:15Z 2020-11-27T12:03:15Z CONTRIBUTOR

It was our pleasure. Thanks @mathause for all the help! :)

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   plot tests: ensure all figures are closed 748379763
734759599 https://github.com/pydata/xarray/pull/4600#issuecomment-734759599 https://api.github.com/repos/pydata/xarray/issues/4600 MDEyOklzc3VlQ29tbWVudDczNDc1OTU5OQ== mathause 10194086 2020-11-27T10:17:32Z 2020-11-27T10:17:32Z MEMBER

Ok. let's get this in before the release. Thanks again @Nirupamkn @yashsaboo, happy to have you as contributors!

 
{
    "total_count": 3,
    "+1": 1,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 2,
    "rocket": 0,
    "eyes": 0
}
   plot tests: ensure all figures are closed 748379763
734498551 https://github.com/pydata/xarray/pull/4600#issuecomment-734498551 https://api.github.com/repos/pydata/xarray/issues/4600 MDEyOklzc3VlQ29tbWVudDczNDQ5ODU1MQ== yashsaboo 24938400 2020-11-26T22:37:36Z 2020-11-26T22:37:36Z CONTRIBUTOR

Thanks @max-sixty!! We have updated the whats-new file too now.

 
{
    "total_count": 1,
    "+1": 1,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   plot tests: ensure all figures are closed 748379763
734394984 https://github.com/pydata/xarray/pull/4600#issuecomment-734394984 https://api.github.com/repos/pydata/xarray/issues/4600 MDEyOklzc3VlQ29tbWVudDczNDM5NDk4NA== max-sixty 5635139 2020-11-26T16:33:44Z 2020-11-26T16:33:44Z MEMBER

Yes exactly, under "Internal Changes"

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   plot tests: ensure all figures are closed 748379763
734088434 https://github.com/pydata/xarray/pull/4600#issuecomment-734088434 https://api.github.com/repos/pydata/xarray/issues/4600 MDEyOklzc3VlQ29tbWVudDczNDA4ODQzNA== Nirupamkn 59981777 2020-11-26T05:46:37Z 2020-11-26T05:46:37Z NONE

@Nirupamkn @yashsaboo do you want to add an entry to the whatsnew file?

Yeah sure! @max-sixty Is there any specific procedure to do it? or Just have to write 2 lines writeup at the end?

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   plot tests: ensure all figures are closed 748379763
734087493 https://github.com/pydata/xarray/pull/4600#issuecomment-734087493 https://api.github.com/repos/pydata/xarray/issues/4600 MDEyOklzc3VlQ29tbWVudDczNDA4NzQ5Mw== max-sixty 5635139 2020-11-26T05:43:20Z 2020-11-26T05:43:20Z MEMBER

@Nirupamkn @yashsaboo do you want to add an entry to the whatsnew file?

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   plot tests: ensure all figures are closed 748379763
734080492 https://github.com/pydata/xarray/pull/4600#issuecomment-734080492 https://api.github.com/repos/pydata/xarray/issues/4600 MDEyOklzc3VlQ29tbWVudDczNDA4MDQ5Mg== Nirupamkn 59981777 2020-11-26T05:19:58Z 2020-11-26T05:20:32Z NONE

This closes all figures at the end of the tests. I'll merge in a few days to see if others have comments.

I changed the title of the PR. Let me know if you object.

@mathause @keewis Thanks a lot for all your feedback! I and @yashsaboo enjoyed working on this! And it was our first open source contribution! Hope to contribute more!

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   plot tests: ensure all figures are closed 748379763
733990530 https://github.com/pydata/xarray/pull/4600#issuecomment-733990530 https://api.github.com/repos/pydata/xarray/issues/4600 MDEyOklzc3VlQ29tbWVudDczMzk5MDUzMA== max-sixty 5635139 2020-11-25T23:31:25Z 2020-11-25T23:31:25Z MEMBER

Thanks @yashsaboo ! Great to have you as a contributor!

 
{
    "total_count": 2,
    "+1": 2,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   plot tests: ensure all figures are closed 748379763
733940666 https://github.com/pydata/xarray/pull/4600#issuecomment-733940666 https://api.github.com/repos/pydata/xarray/issues/4600 MDEyOklzc3VlQ29tbWVudDczMzk0MDY2Ng== yashsaboo 24938400 2020-11-25T20:53:06Z 2020-11-25T20:53:06Z CONTRIBUTOR

Sounds good @mathause. Thanks again! :)

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   plot tests: ensure all figures are closed 748379763
733343703 https://github.com/pydata/xarray/pull/4600#issuecomment-733343703 https://api.github.com/repos/pydata/xarray/issues/4600 MDEyOklzc3VlQ29tbWVudDczMzM0MzcwMw== yashsaboo 24938400 2020-11-25T00:16:20Z 2020-11-25T00:16:20Z CONTRIBUTOR

Thanks, @keewis for bringing that point up. We were aware of test_plot_transposed_nondim_coord polluter but were still investigating the test cases it was polluting. FYI, you can try this --random-order-seed=846619. Please feel free to give more suggestions. Highly appreciate it! @mathause, we have implemented all of the above changes. Thanks a lot again! :)

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   plot tests: ensure all figures are closed 748379763
733235217 https://github.com/pydata/xarray/pull/4600#issuecomment-733235217 https://api.github.com/repos/pydata/xarray/issues/4600 MDEyOklzc3VlQ29tbWVudDczMzIzNTIxNw== mathause 10194086 2020-11-24T21:10:16Z 2020-11-24T21:10:16Z MEMBER

Good catch! If you haven't lost patience, there are two more things that leave open figures: test_plot_transposed_nondim_coord and some of the tests in TestDiscreteColorMap.

  1. To fix the first both of these lines need to be enclosed in a figure_context (each line separately):

https://github.com/pydata/xarray/blob/f1e2f9d510a63480f4dd7917083cd982ac4d451d/xarray/tests/test_plot.py#L2431-L2432

  1. To fix the second you can add the following code

python yield # Remove all matplotlib figures plt.close("all") after the following line:

https://github.com/pydata/xarray/blob/f1e2f9d510a63480f4dd7917083cd982ac4d451d/xarray/tests/test_plot.py#L964

And you should give yourselves credit in what's new. That's now more than a one-liner ;-)

Just as an addendum. The figures in the PlotTestCase class are autoclosed by

https://github.com/pydata/xarray/blob/f1e2f9d510a63480f4dd7917083cd982ac4d451d/xarray/tests/test_plot.py#L128-L133

but only if no more than one figure is opened per method. 

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   plot tests: ensure all figures are closed 748379763
733216560 https://github.com/pydata/xarray/pull/4600#issuecomment-733216560 https://api.github.com/repos/pydata/xarray/issues/4600 MDEyOklzc3VlQ29tbWVudDczMzIxNjU2MA== keewis 14808389 2020-11-24T20:30:00Z 2020-11-24T20:30:00Z MEMBER

how thorough should this be? If I understand it correctly, there are still a few calls left, e.g. in test_plot_transposed_nondim_coord (hidden by getattr calls).

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   plot tests: ensure all figures are closed 748379763
732469302 https://github.com/pydata/xarray/pull/4600#issuecomment-732469302 https://api.github.com/repos/pydata/xarray/issues/4600 MDEyOklzc3VlQ29tbWVudDczMjQ2OTMwMg== yashsaboo 24938400 2020-11-23T22:44:21Z 2020-11-23T22:44:21Z CONTRIBUTOR

Thanks a lot again for the feedback. Appreciate it! We have made the necessary changes.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   plot tests: ensure all figures are closed 748379763
732445905 https://github.com/pydata/xarray/pull/4600#issuecomment-732445905 https://api.github.com/repos/pydata/xarray/issues/4600 MDEyOklzc3VlQ29tbWVudDczMjQ0NTkwNQ== mathause 10194086 2020-11-23T21:48:28Z 2020-11-23T21:48:28Z MEMBER 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   plot tests: ensure all figures are closed 748379763
732409466 https://github.com/pydata/xarray/pull/4600#issuecomment-732409466 https://api.github.com/repos/pydata/xarray/issues/4600 MDEyOklzc3VlQ29tbWVudDczMjQwOTQ2Ng== yashsaboo 24938400 2020-11-23T20:31:21Z 2020-11-23T20:31:21Z CONTRIBUTOR

Thanks a lot for the feedback. We have addressed all the above issues and made the necessary changes. Please let us know your thoughts on these new changes.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   plot tests: ensure all figures are closed 748379763
732041825 https://github.com/pydata/xarray/pull/4600#issuecomment-732041825 https://api.github.com/repos/pydata/xarray/issues/4600 MDEyOklzc3VlQ29tbWVudDczMjA0MTgyNQ== mathause 10194086 2020-11-23T09:36:25Z 2020-11-23T09:36:25Z MEMBER

Thanks for looking into this. Yes, closing the figure at the beginning is brittle, unless every test does it. However, it cannot be done at the end because else the figure is not closed if there is an error in the test (and the figure is still open at the start of the next test).

My preference is to enclose the test in a figure_context as already done for some of the tests: https://github.com/pydata/xarray/blob/9533c92c675a30b8c9803da8d5c9bc529763f3eb/xarray/tests/test_plot.py#L2456-L2457

figure_context is a small context manager that wraps a try: ... finally block, closing all open figures. https://github.com/pydata/xarray/blob/9533c92c675a30b8c9803da8d5c9bc529763f3eb/xarray/tests/test_plot.py#L49

I think all plt.clf() instances should be replaced with figure_context. I missed those in my PR (#4365). Let us know if you are up to that.

cc @dcherian

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   plot tests: ensure all figures are closed 748379763

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]);