home / github

Menu
  • Search all tables
  • GraphQL API

issue_comments

Table actions
  • GraphQL API for issue_comments

15 rows where issue = 303103716 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 6

  • Zac-HD 7
  • shoyer 3
  • tacaswell 2
  • jklymak 1
  • max-sixty 1
  • fmaussion 1

author_association 3

  • CONTRIBUTOR 8
  • MEMBER 5
  • NONE 2

issue 1

  • Starter property-based test suite · 15 ✖
id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
374583488 https://github.com/pydata/xarray/pull/1972#issuecomment-374583488 https://api.github.com/repos/pydata/xarray/issues/1972 MDEyOklzc3VlQ29tbWVudDM3NDU4MzQ4OA== fmaussion 10050469 2018-03-20T12:42:37Z 2018-03-20T12:42:37Z MEMBER

Thanks @Zac-HD !

{
    "total_count": 2,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 1,
    "confused": 0,
    "heart": 1,
    "rocket": 0,
    "eyes": 0
}
  Starter property-based test suite 303103716
374573132 https://github.com/pydata/xarray/pull/1972#issuecomment-374573132 https://api.github.com/repos/pydata/xarray/issues/1972 MDEyOklzc3VlQ29tbWVudDM3NDU3MzEzMg== Zac-HD 12229877 2018-03-20T12:03:13Z 2018-03-20T12:03:13Z CONTRIBUTOR

I would say we should merge it now and iterate in future PRs.

Merge away then!

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Starter property-based test suite 303103716
373954379 https://github.com/pydata/xarray/pull/1972#issuecomment-373954379 https://api.github.com/repos/pydata/xarray/issues/1972 MDEyOklzc3VlQ29tbWVudDM3Mzk1NDM3OQ== shoyer 1217238 2018-03-17T21:35:52Z 2018-03-17T21:36:37Z MEMBER

blockers to merging this as-is?

This looks pretty good to me in its current state. I would say we should merge it now and iterate in future PRs.

other APIs you think it would be reasonably easy to write property tests for? Here's a nice list of properties to test 😄

Almost anywhere where we currently make heavy use of parametrize would be a good candidate. Some other possibilities:

  • Consistency with pandas for groupby/rolling aggregations.
  • Roundtrip writing/reading data to netCDF. There are a couple of known exceptions (e.g., dtypes not supported by netCDF and MultiIndex) but otherwise every xarray object should be serializable to netCDF and back without data loss.
  • Roundtrip to/from pandas Series/DataFrame with to_series()/to_dataframe()/to_xarray().
  • Indexing consistency tests for backends: all indexing operations should be supported consistently on data accessed from any backend.
  • NumPy vs Dask: any operation on dask arrays should be consistent with the operation on numpy arrays (e.g., f(xarray_obj.chunk()).compute() == f(xarray_obj)).
  • Indexing followed by xarray.concat: should get back the same result.
  • Binary arithmetic on xarray objects with Python operators (+, -, etc) and NumPy ufuncs (np.add, np.subtract, etc).
{
    "total_count": 1,
    "+1": 1,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Starter property-based test suite 303103716
373545890 https://github.com/pydata/xarray/pull/1972#issuecomment-373545890 https://api.github.com/repos/pydata/xarray/issues/1972 MDEyOklzc3VlQ29tbWVudDM3MzU0NTg5MA== Zac-HD 12229877 2018-03-15T22:41:20Z 2018-03-15T22:41:20Z CONTRIBUTOR

@shoyer & @fmaussion - I've just given up on the plotting tests as being more effort than they're worth. Are there any:

  • blockers to merging this as-is?
  • other APIs you think it would be reasonably easy to write property tests for? Here's a nice list of properties to test :smile:
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Starter property-based test suite 303103716
371819126 https://github.com/pydata/xarray/pull/1972#issuecomment-371819126 https://api.github.com/repos/pydata/xarray/issues/1972 MDEyOklzc3VlQ29tbWVudDM3MTgxOTEyNg== tacaswell 199813 2018-03-09T13:58:15Z 2018-03-09T13:58:15Z NONE

Set the backend to Agg on travis as you don't have a xserever running. You probably want to manually force a draw as well.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Starter property-based test suite 303103716
371796211 https://github.com/pydata/xarray/pull/1972#issuecomment-371796211 https://api.github.com/repos/pydata/xarray/issues/1972 MDEyOklzc3VlQ29tbWVudDM3MTc5NjIxMQ== Zac-HD 12229877 2018-03-09T12:11:02Z 2018-03-09T12:11:02Z CONTRIBUTOR

@jklymak - I'm getting RuntimeError: Invalid DISPLAY variable in the Qt backend now that I've added plt.clf(). It works on my machine now (:tada:, thanks!) - any suggestions for Travis?

(I'm also getting Zarr errors, but I assume those will go away soon as I didn't cause them)

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Starter property-based test suite 303103716
371722392 https://github.com/pydata/xarray/pull/1972#issuecomment-371722392 https://api.github.com/repos/pydata/xarray/issues/1972 MDEyOklzc3VlQ29tbWVudDM3MTcyMjM5Mg== Zac-HD 12229877 2018-03-09T06:05:46Z 2018-03-09T06:05:46Z CONTRIBUTOR

...that also explains why I was having trouble reproducing the error, whoops. I'll see how it goes with those problems excluded later tonight!

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Starter property-based test suite 303103716
371718795 https://github.com/pydata/xarray/pull/1972#issuecomment-371718795 https://api.github.com/repos/pydata/xarray/issues/1972 MDEyOklzc3VlQ29tbWVudDM3MTcxODc5NQ== jklymak 1562854 2018-03-09T05:38:23Z 2018-03-09T05:38:23Z CONTRIBUTOR

As pointed out on the matplotlib gitter:

If you run

```python import numpy as np import xarray as xr import matplotlib.pyplot as plt

for i in range(200): xr.DataArray(np.array([[0, 0], [0, 0]], dtype=np.uint8)).plot.pcolormesh() at step 165 you will get: File "/Users/jklymak/matplotlib/lib/matplotlib/figure.py", line 236, in update raise ValueError('left cannot be >= right') ValueError: left cannot be >= right ``` Why? Because you have made a plot that if it displays looks like:

Are you sure your test isn't doing something similar? At some point there just isn't room for more colorbars! Adding a plt.clf() can cure the problem.

Its also is possible you are hitting floating point overflows with your test. At some point Matplotlib needs to be able to manipulate the data that comes in, and if you operate near the maximum number your data type can handle, you'll have problems. Just like you would if you just did

python a = 2*xr.DataArray(np.array([[0, 0], [0, 1e308]])) you will get: /Users/jklymak/anaconda3/envs/matplotlibdev/lib/python3.6/site-packages/xarray/core/variable.py:1165: RuntimeWarning: overflow encountered in multiply So maybe your hypothesis tester could be constrained to stay away from floating point overflows?

Matplotlib indeed has flaws and quirks, but if you are finding bugs it would be good to isolate them.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Starter property-based test suite 303103716
371691628 https://github.com/pydata/xarray/pull/1972#issuecomment-371691628 https://api.github.com/repos/pydata/xarray/issues/1972 MDEyOklzc3VlQ29tbWVudDM3MTY5MTYyOA== max-sixty 5635139 2018-03-09T02:18:13Z 2018-03-09T02:18:13Z MEMBER

If we don't want to trigger by default, we can do something like this and require passing this to run them:

pytest --property-tests

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Starter property-based test suite 303103716
371689343 https://github.com/pydata/xarray/pull/1972#issuecomment-371689343 https://api.github.com/repos/pydata/xarray/issues/1972 MDEyOklzc3VlQ29tbWVudDM3MTY4OTM0Mw== Zac-HD 12229877 2018-03-09T02:05:18Z 2018-03-09T02:05:39Z CONTRIBUTOR

@shoyer - that depends mostly on whether you want to run these tests as part of a standard run. A test-time dependency on Hypothesis is very cheap compared to the other dev dependencies, so I'd think more about the impact on eg CI resources than contributors.

Upside is more power and coverage of edge-cases with odd data; downside is that they take a lot longer by virtue of trying hundreds of examples, and in this case also having to generate arrays takes a while (~log(elements) average via sparse filling).


@tacaswell - I would be delighted to write a test suite like this for matplotlib! The only reason I haven't is because I thought it would be rude to report so many bugs that I don't have time to help fix. If we can get a group project going though I'd be very enthusiastic 😄

  • For this suite I was passing in 2D arrays of unsigned ints, signed ints, or floats (in all sizes), with edge sizes between 2 and 10.
  • The failing outputs were reported as 2x2 arrays (~30 times), or 3x2 (once - there's a cap on number of shrinks that I think I hit there).
  • Values tended to be either uniform small integers - 0 or 1, dtype int8 or uint8; or for some an array of floats with one corner zero and the others very large.

I didn't keep the exact tracebacks, but I remember seeing many come from overflow in tick spacing calculations. Again, happy to write a test suite and make more detailed reports upstream if people want to fix this - in which case let's open an issue there!

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Starter property-based test suite 303103716
371681439 https://github.com/pydata/xarray/pull/1972#issuecomment-371681439 https://api.github.com/repos/pydata/xarray/issues/1972 MDEyOklzc3VlQ29tbWVudDM3MTY4MTQzOQ== tacaswell 199813 2018-03-09T01:20:44Z 2018-03-09T01:20:44Z NONE

What do the failing data sets look like? Does it get easier or harder to find failures if you go up to 10x10? What sort of exceptions are you getting?

You can shove a fair amount of configuration in to pytest.ini to make these opt-in.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Starter property-based test suite 303103716
371672811 https://github.com/pydata/xarray/pull/1972#issuecomment-371672811 https://api.github.com/repos/pydata/xarray/issues/1972 MDEyOklzc3VlQ29tbWVudDM3MTY3MjgxMQ== shoyer 1217238 2018-03-09T00:31:07Z 2018-03-09T00:31:07Z MEMBER

One thing that comes to mind is organization... would it make sense to put this alongside the current xarray tests, e.g., have xarray/tests/unit and xarray/tests/property?

I guess one downside of this would be that it could change how we need to invoke py.test by default, if we don't want to trigger all the property based tests.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Starter property-based test suite 303103716
371671971 https://github.com/pydata/xarray/pull/1972#issuecomment-371671971 https://api.github.com/repos/pydata/xarray/issues/1972 MDEyOklzc3VlQ29tbWVudDM3MTY3MTk3MQ== Zac-HD 12229877 2018-03-09T00:26:13Z 2018-03-09T00:26:13Z CONTRIBUTOR

This looks like a great start to me -- thank you!

You're welcome! Same questions as above though, plus "is there anything else you need in this initial PR?". If not, can we merge it

It's impressive that it's possible to break every plotting type with matplotlib :).

As much as I love matplotlib, it's a steaming pile of hacks and I want to avoid it more than I want it cleaned up 😥 (entirely because the process is dysfunctional, not the code)

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Starter property-based test suite 303103716
371593519 https://github.com/pydata/xarray/pull/1972#issuecomment-371593519 https://api.github.com/repos/pydata/xarray/issues/1972 MDEyOklzc3VlQ29tbWVudDM3MTU5MzUxOQ== shoyer 1217238 2018-03-08T19:17:48Z 2018-03-08T19:17:48Z MEMBER

This looks like a great start to me -- thank you!

It's impressive that it's possible to break every plotting type with matplotlib :).

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Starter property-based test suite 303103716
371403712 https://github.com/pydata/xarray/pull/1972#issuecomment-371403712 https://api.github.com/repos/pydata/xarray/issues/1972 MDEyOklzc3VlQ29tbWVudDM3MTQwMzcxMg== Zac-HD 12229877 2018-03-08T07:29:18Z 2018-03-08T07:29:18Z CONTRIBUTOR

Ping @fmaussion / @shoyer - would love your opinions on this, including high-value targets to test.

(btw Appveyor had an internal error; build is otherwise green)

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Starter property-based test suite 303103716

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 15.445ms · About: xarray-datasette