Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

⚠️ Nightly upstream-dev CI failed ⚠️ #7707

Closed
github-actions bot opened this issue Apr 1, 2023 · 8 comments · Fixed by #7876
Closed

⚠️ Nightly upstream-dev CI failed ⚠️ #7707

github-actions bot opened this issue Apr 1, 2023 · 8 comments · Fixed by #7876
Labels
CI Continuous Integration tools

Comments

@github-actions
Copy link
Contributor

github-actions bot commented Apr 1, 2023

Workflow Run URL

Python 3.10 Test Summary

@github-actions github-actions bot added the CI Continuous Integration tools label Apr 1, 2023
@dcherian
Copy link
Contributor

dcherian commented Apr 4, 2023

Oh wow, we're down to mostly Zarr failures!

cc @jhamman

@keewis
Copy link
Collaborator

keewis commented Apr 4, 2023

it seems the tests segfaulted again. Not sure which test exactly is causing that, though.

@spencerkclark
Copy link
Member

spencerkclark commented Apr 5, 2023

I think it is fine that CFTimeIndex.to_datetimeindex() no longer raises an error for out-of-nanosecond-precision range dates, so we can simply relax that test for pandas versions greater than or equal to 2 and update its docstring.

In practice I don't think this will come up very often (we can address later if we want probably), but one subtle issue there is that prior to October 15th, 1582, the proleptic Gregorian calendar and the "standard" calendar are not equivalent, so we may want to update how we warn when converting between calendars. The "standard" calendar according to the CF Conventions is a mixed Julian/Gregorian calendar, which uses a Julian calendar prior to 1582-10-15 and a Gregorian calendar after.

In cftime the DatetimeGregorian object conforms to this definition, and is what is created if you provide "standard" as the calendar argument to num2date:

>>> cftime.num2date([0, 1], units="days since 1582-10-04", calendar="standard")
array([cftime.DatetimeGregorian(1582, 10, 4, 0, 0, 0, 0, has_year_zero=False),
       cftime.DatetimeGregorian(1582, 10, 15, 0, 0, 0, 0, has_year_zero=False)],
      dtype=object)
>>> cftime.num2date([0, 1], units="days since 1582-10-04", calendar="proleptic_gregorian")
array([cftime.DatetimeProlepticGregorian(1582, 10, 4, 0, 0, 0, 0, has_year_zero=True),
       cftime.DatetimeProlepticGregorian(1582, 10, 5, 0, 0, 0, 0, has_year_zero=True)],
      dtype=object)

I'll need to think more about how to handle the test_should_cftime_be_used_source_outside_range failure; I'm not sure if we're ready to handle changing the behavior of this until we fully address #7493.

@keewis
Copy link
Collaborator

keewis commented Apr 16, 2023

flaky segfaults aside, we're down to just the zarr v3 tests, a flaky cdms2 test, and a test related to pint (though that one appears to be an upstream issue – not entirely sure, though).

@Illviljan
Copy link
Contributor

Lots of pint errors with version 0.21, @keewis. I think pint 0.20.1 worked well?

@keewis
Copy link
Collaborator

keewis commented May 5, 2023

pint<0.21 should work, and I'm looking into how this could be fixed, see hgrecco/pint#1660 and hgrecco/pint#1749. For the latter we might have to mark the "pint wrapping dask" test as requiring pint>=0.21 and make it use an explicit registry.

@jhamman
Copy link
Member

jhamman commented May 7, 2023

See #7825 for a PR fixing the outstanding Zarr V3 failures.

@keewis
Copy link
Collaborator

keewis commented May 20, 2023

with #7855 and the recent change to pint we're finally down to just two test failures (and a whole lot of warnings):

xarray/tests/test_dataarray.py::TestDataArray::test_to_and_from_cdms2_sgrid: ValueError: operands could not be broadcast together with shapes (3,) (4,)
xarray/tests/test_ufuncs.py::test_unary: AssertionError: assert (<class 'int'> is <class 'numpy.float64'> or 1.0 == 0.9999999999999999)

The first one looks like cdms2 is incompatible with a change in numpy>=1.25. Not sure if we can do anything about that, especially since there's a big warning on the cdms2 repo stating that the package is going to be retired / archived by the end of this year... I guess we should start thinking about removing cdms2 support?

The second looks like a precision issue, which we should be able to resolve by using assert_allclose instead... not sure, though, especially given numpy/numpy#23773. Otherwise we could just defer to whatever numpy is doing (of course, that assumes that full_like works properly, which might not be a good idea for a unit test):

@pytest.mark.parametrize(
    "a",
    [
        xr.Variable(["x"], [0, 0]),
        xr.DataArray([0, 0], dims="x"),
        xr.Dataset({"y": ("x", [0, 0])}),
    ],
)
def test_unary(a):
    fill_value = np.cos(0)

    expected = xr.full_like(a, fill_value=fill_value, dtype=fill_value.dtype)
    actual = np.cos(a)

    assert_identical(actual, expected)

Edit: if relying on full_like turns out to be a concern, maybe we could use "copy + assign" instead?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration tools
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants