issue_comments

Table actions

8 rows where author_association = "MEMBER" and issue = 1020407925 sorted by updated_at descending

✎ View and edit SQL

This data as json, CSV (advanced)

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

user 3

issue 1

  • remove requirement for setuptools.pkg_resources · 8 ✖

author_association 1

  • MEMBER · 8 ✖
id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
956286144 https://github.com/pydata/xarray/pull/5845#issuecomment-956286144 https://api.github.com/repos/pydata/xarray/issues/5845 IC_kwDOAMm_X844_8TA dcherian 2448579 2021-11-01T14:33:19Z 2021-11-01T14:33:19Z MEMBER

Thanks @marscher This is a great change.

I see this is your first PR. Welcome to Xarray!

 
{
    "total_count": 1,
    "+1": 1,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   remove requirement for setuptools.pkg_resources 1020407925
954886300 https://github.com/pydata/xarray/pull/5845#issuecomment-954886300 https://api.github.com/repos/pydata/xarray/issues/5845 IC_kwDOAMm_X8446mic dcherian 2448579 2021-10-29T16:35:43Z 2021-10-29T16:35:43Z MEMBER

Test failure is real xarray/__init__.py:34: error: Name "_version" already defined (possibly by an import) [no-redef] Found 1 error in 1 file (checked 145 source files)

I think we just revert the last commit and merge...

 
{
    "total_count": 1,
    "+1": 1,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   remove requirement for setuptools.pkg_resources 1020407925
954246645 https://github.com/pydata/xarray/pull/5845#issuecomment-954246645 https://api.github.com/repos/pydata/xarray/issues/5845 IC_kwDOAMm_X8444KX1 keewis 14808389 2021-10-28T21:45:31Z 2021-10-28T21:47:35Z MEMBER

thanks for the PR, @marscher, and especially for being patient with us/me.

if the CI still passes without the explicit mention of setuptools in the CI environments this should be ready for merging. It would probably be good to review the change to the dependency policy, though (do we need to list setuptools even if we only depend on it at build time?)

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   remove requirement for setuptools.pkg_resources 1020407925
953773679 https://github.com/pydata/xarray/pull/5845#issuecomment-953773679 https://api.github.com/repos/pydata/xarray/issues/5845 IC_kwDOAMm_X8442W5v keewis 14808389 2021-10-28T11:55:24Z 2021-10-28T11:55:24Z MEMBER

the CI passes, too. Now we just need an entry in whats-new.rst saying that the runtime dependency on setuptools was dropped, and we might also want to remove the explicit mention of setuptools in the CI files

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   remove requirement for setuptools.pkg_resources 1020407925
953085452 https://github.com/pydata/xarray/pull/5845#issuecomment-953085452 https://api.github.com/repos/pydata/xarray/issues/5845 IC_kwDOAMm_X844zu4M keewis 14808389 2021-10-27T16:15:43Z 2021-10-27T16:45:37Z MEMBER

thanks, @dopplershift. We agreed on doing that in the last meeting, and to then drop the new dependency together with python=3.7 in December. That means that if we can figure out the remaining issues we can include this in the next release (which will be very soon!)

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   remove requirement for setuptools.pkg_resources 1020407925
940127873 https://github.com/pydata/xarray/pull/5845#issuecomment-940127873 https://api.github.com/repos/pydata/xarray/issues/5845 IC_kwDOAMm_X844CTaB keewis 14808389 2021-10-11T15:23:06Z 2021-10-11T15:24:43Z MEMBER

I might be confusing something, but I thought preprocessing selectors were executed at build time? So if you build the noarch: python package in an environment that has python=3.7 you will keep the importlib_metadata while in a different one with python=3.8 it will be removed?

The easiest way to figure this out would be to build in a py38 environment, install in a py37 environment and then try to import it (i.e. trigger the code path that uses importlib.metadata). If that works, then it would be fine to merge now.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   remove requirement for setuptools.pkg_resources 1020407925
939371374 https://github.com/pydata/xarray/pull/5845#issuecomment-939371374 https://api.github.com/repos/pydata/xarray/issues/5845 IC_kwDOAMm_X843_atu keewis 14808389 2021-10-09T22:41:15Z 2021-10-10T16:40:21Z MEMBER

is there any reason not to merge this?

the issue is that with conda we can either always install importlib_metadata or not at all: conda doesn't support noarch_python packages with preprocessing selectors (see the comments around https://github.com/pydata/xarray/issues/4295#issuecomment-667657310 for more discussion).

importlib.metadata is included in python>=3.8, which we can officially require from end of December this year.

All of this means that if we want to merge this now we have to decide between not releasing until we can drop python=3.7 or always installing importlib_metadata (which is what the pint feedstock does). However, I guess the cleanest way would be to wait until we can require python>=3.8.

 
{
    "total_count": 1,
    "+1": 1,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   remove requirement for setuptools.pkg_resources 1020407925
939368646 https://github.com/pydata/xarray/pull/5845#issuecomment-939368646 https://api.github.com/repos/pydata/xarray/issues/5845 IC_kwDOAMm_X843_aDG max-sixty 5635139 2021-10-09T22:11:31Z 2021-10-09T22:11:31Z MEMBER

Thanks for the PR @marscher

I don't know this area well — assuming this cuts the import time down considerably, is there any reason not to merge this?

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   remove requirement for setuptools.pkg_resources 1020407925

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