home / github

Menu
  • Search all tables
  • GraphQL API

issue_comments

Table actions
  • GraphQL API for issue_comments

5 rows where user = 47244312 sorted by updated_at descending

✎ View and edit SQL

This data as json, CSV (advanced)

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

issue 4

  • Support non-string dimension/variable names 2
  • Scalar coords vs. concat 1
  • Many methods are broken (e.g., concat/stack/sortby) when using repeated dimensions 1
  • xarray.concat docstring confuses PyCharm type detection 1

user 1

  • gimperiale · 5 ✖

author_association 1

  • CONTRIBUTOR 5
id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
528920519 https://github.com/pydata/xarray/issues/1378#issuecomment-528920519 https://api.github.com/repos/pydata/xarray/issues/1378 MDEyOklzc3VlQ29tbWVudDUyODkyMDUxOQ== gimperiale 47244312 2019-09-06T16:22:12Z 2019-09-06T16:22:12Z CONTRIBUTOR

I'm not too fond of having multiple dimensions with the same name because, whenever you need to operate on one but not the other, you have little to no choice but revert to positional indexing.

Consider also how many methods expect either **kwargs or a dict-like parameter with the dimension or variable names as the keys. I would not be surprised to find that many API design choices fall apart in the face of this use case.

Also, having two non positional (as it should always be in xarray!) dimensions with the same name only makes sense when modelling symmetric N:N relationships. Two good examples are covariance matrices and the weights for a Dijkstra algorithm.

The problems start when the object represents an asymmetric relationship, e.g: - Cost (for the purpose of graph resolution, so time/money/other) of transportation via river, where going from A->B (downstream) is cheaper than going back from B->A (upstream) - Currency conversion, where EUR->USD is not identical to 1/(USD->EUR) because of arbitrage and illiquidity - In financial Monte Carlo simulations, I had to deal with credit rating transition matrices which define the probability of a company to change its credit rating. In unfavourable market conditions, the chances of being downgraded from AAA to AA are higher than being promoted from AA to AAA.

I could easily come up with many other cases. In case of asymmetric N:N relationships, it is highly desirable to share the same index across multiple dimensions with different names (that would typically convey the direction of the relationship, e.g. "from" and "to").

What if, instead of allowing for duplicate dimensions, we allowed sharing an index across different dimensions?

Something like python river_transport = Dataset( coords={ 'station': ['Kingston', 'Montreal'], 'station_from': ('station', ) 'station_to': ('station', ) }, data_vars={ cost=(('station_from', 'station_to'), [[0, 20], [15, 0]]), } } or, for DataArrays: python river_transport = DataArray( [[0, 20], [15, 0]], dims=('station_from', 'station_to'), coords={ 'station': ['Kingston', 'Montreal'], 'station_from': ('station', ) 'station_to': ('station', ) }, }

Note how this syntax doesn't exist as of today: python 'station_from': ('station', ) 'station_to': ('station', ) From an implementation point of view, I think it could be easily implemented by keeping track of a map of aliases and with some __geitem__ magic. More effort would be needed to convince DataArrays to accept (and not accidentally drop) a coordinate whose dims don't match any of the data variable's.

This design would not resolve the issue of compatibility with NetCDF though. I'd be surprised if the NetCDF designers never came across this - maybe it's a good idea to have a chat with them?

{
    "total_count": 5,
    "+1": 5,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Many methods are broken (e.g., concat/stack/sortby) when using repeated dimensions 222676855
490824656 https://github.com/pydata/xarray/issues/2292#issuecomment-490824656 https://api.github.com/repos/pydata/xarray/issues/2292 MDEyOklzc3VlQ29tbWVudDQ5MDgyNDY1Ng== gimperiale 47244312 2019-05-09T09:13:22Z 2019-05-09T09:13:22Z CONTRIBUTOR

A possible way out would be to open a PEP for "and" and "not" operators in the typing module. That way we could define a "variable-name-like" type and use it throughout the module:

xarray.utils: from typing import AllOf, Hashable, NoneOf VarName = AllOf[Hashable, NoneOf[None, tuple]] Elsewhere: from .utils import VarName def f(x: Union[VarName, Sequence[VarName], None]): if x is None: x = [DEFAULT] elif isinstance(x, VarName): x = [x] elif not isinstance(x, Sequence): raise TypeError('x: expected hashable or sequence of hashables)

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Support non-string dimension/variable names 341643235
490821558 https://github.com/pydata/xarray/issues/2292#issuecomment-490821558 https://api.github.com/repos/pydata/xarray/issues/2292 MDEyOklzc3VlQ29tbWVudDQ5MDgyMTU1OA== gimperiale 47244312 2019-05-09T09:04:21Z 2019-05-09T09:05:48Z CONTRIBUTOR

There are problems with typing. I already mentioned them in #2929 but I'll summarize here.

The vast majority of xarray functions/methods allow for "string or sequence of strings, optional". When you move to "hashable or sequence of hashables, optional", however, you want to specifically avoid tuples, which are both Sequence and Hashable instances.

Most functions currently look like this: if isinstance(x, str): x = [x] elif x is None: x = [DEFAULT] for xi in x: ... After the change they would become: if x is None: x = [DEFAULT] elif isinstance(x, Hashable) and not isinstance(x, tuple): x = [x] for xi in x: ... Or: if x is None: x = [DEFAULT] elif isinstance(x, str) or not isinstance(x, Sequence): x = [x] for xi in x: ... Note how I moved the test for None above. This matters, because isinstance(None, Hashable) returns True. This is very error-prone and expensive to maintain, which will very easily cause beginner contributors to introduce bugs. Every test that currently runs three use cases, one for None, one for str and another for a sequence of str, will now be forced to be expanded to SIX test cases:

  1. str
  2. tuple (hashable sequence) of str
  3. list (non-hashable sequence) of str
  4. enum (non-str, non-sequence hashable)
  5. sequence of non-sortable hashables
  6. None

One way to mitigate it would be to have an helper function, which would be invoked everywhere around the codebase, and then religiously make sure that the helper function is always used. _no_default = [object()] def ensure_sequence(name: str, x: Union[Hashable, Sequence[Hashable]], default: Sequence[Hashable] = _no_default) -> Sequence[Hashable]: if x is None: if default is _no_default: raise ValueError(name + ' must be explicitly defined') return default if isinstance(x, Sequence) and not isinstance(x, str): return x if isinstance(x, Hashable): return [x] raise TypeError(name + ' must be a Hashable or Sequence of Hashable') You would still be forced to implement the test for non-sortable hashables, though.


A completely separate problem with typing is that I expect a huge amount of xarray users to just assume variable names and dims are always strings. They'll have things like for k, v in ds.data_vars: if k.startswith('foo'): ... or [dim for dim in da.dims if "foo" in dim] The above will fill the mypy output with errors as soon as xarray becomes integrated in mypy (#2929), and the user will have to go through a lot of explicitly forcing dims and variable names to str, even if in their project all dims and variables names are always str.


The final problem is that integers are Hashables, and there's a wealth of cases in xarray where there is special logic that dynamically treats ints as positional indices.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Support non-string dimension/variable names 341643235
480303272 https://github.com/pydata/xarray/issues/2869#issuecomment-480303272 https://api.github.com/repos/pydata/xarray/issues/2869 MDEyOklzc3VlQ29tbWVudDQ4MDMwMzI3Mg== gimperiale 47244312 2019-04-05T14:47:29Z 2019-04-05T14:47:29Z CONTRIBUTOR

@max-sixty because of this https://github.com/pydata/xarray/blob/c33dab237a0aca316b7f6d4ba10857829a895a25/xarray/core/combine.py#L73

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  xarray.concat docstring confuses PyCharm type detection 429685866
459770570 https://github.com/pydata/xarray/issues/1151#issuecomment-459770570 https://api.github.com/repos/pydata/xarray/issues/1151 MDEyOklzc3VlQ29tbWVudDQ1OTc3MDU3MA== gimperiale 47244312 2019-02-01T15:58:21Z 2019-02-01T15:58:21Z CONTRIBUTOR

This issue is still valid as of xarray 0.11.0

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Scalar coords vs. concat 193294569

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]);
Powered by Datasette · Queries took 14.027ms · About: xarray-datasette