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

Fix min_deps_check; revert to support numpy=1.14 and pandas=0.24 #4178

Closed
wants to merge 12 commits into from
19 changes: 9 additions & 10 deletions ci/min_deps_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,21 +141,20 @@ def process_pkg(
policy_months = POLICY_MONTHS.get(pkg, POLICY_MONTHS_DEFAULT)
policy_published = datetime.now() - timedelta(days=policy_months * 30)

policy_major = req_major
policy_minor = req_minor
policy_published_actual = req_published
for (major, minor), published in reversed(sorted(versions.items())):
if published < policy_published:
break
Comment on lines -144 to -149
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not entirely sure why this was broken, but the new logic is significantly simpler so I'm more confident it's correct. In particular, it determines the policy versions without using the currently required versions as defaults.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An alternative implement of the original policy that is possibly clearer:

Suggested change
policy_major = req_major
policy_minor = req_minor
policy_published_actual = req_published
for (major, minor), published in reversed(sorted(versions.items())):
if published < policy_published:
break
# "the minor version (X.Y) initially published no more than N months ago"
policy_major, policy_minor = min(
(version for version, date in versions.items() if date > policy_published),
default=(req_major, req_minor),
)

policy_major = major
policy_minor = minor
policy_published_actual = published
policy_major, policy_minor = max(
version for version, date in versions.items() if date < policy_published
)
policy_published_actual = versions[policy_major, policy_minor]

if (req_major, req_minor) < (policy_major, policy_minor):
status = "<"
elif (req_major, req_minor) > (policy_major, policy_minor):
status = "> (!)"
error("Package is too new: " + pkg)
error(
f"Package is too new: {pkg}={req_major}.{req_minor} was "
f"published on {versions[req_major, req_minor]:%Y-%m-%d} "
f"(within the past {policy_months} months)"
)
else:
status = "="

Expand Down
4 changes: 2 additions & 2 deletions ci/requirements/py36-bare-minimum.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@ dependencies:
- pytest
- pytest-cov
- pytest-env
- numpy=1.15
- pandas=0.25
- numpy=1.14
- pandas=0.24
- setuptools=41.2
4 changes: 2 additions & 2 deletions ci/requirements/py36-min-all-deps.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ dependencies:
- nc-time-axis=1.2
- netcdf4=1.4
- numba=0.44
- numpy=1.15
- pandas=0.25
- numpy=1.14
- pandas=0.24
# - pint # See py36-min-nep18.yml
- pip
- pseudonetcdf=3.0
Expand Down
4 changes: 2 additions & 2 deletions doc/installing.rst
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ Required dependencies

- Python (3.6 or later)
- setuptools
- `numpy <http://www.numpy.org/>`__ (1.15 or later)
- `pandas <http://pandas.pydata.org/>`__ (0.25 or later)
- `numpy <http://www.numpy.org/>`__ (1.14 or later)
- `pandas <http://pandas.pydata.org/>`__ (0.24 or later)

.. _optional-dependencies:

Expand Down
4 changes: 2 additions & 2 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ zip_safe = False # https://mypy.readthedocs.io/en/latest/installed_packages.htm
include_package_data = True
python_requires = >=3.6
install_requires =
numpy >= 1.15
pandas >= 0.25
numpy >= 1.14
pandas >= 0.24
setuptools >= 41.2 # For pkg_resources
setup_requires =
setuptools >= 41.2
Expand Down
12 changes: 11 additions & 1 deletion xarray/core/nputils.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import numpy as np
import pandas as pd
from numpy.core.multiarray import normalize_axis_index

try:
import bottleneck as bn
Expand All @@ -14,6 +13,17 @@
_USE_BOTTLENECK = False


def normalize_axis_index(data, axis):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this because I noticed it when manually reverting the quantile change from https://github.com/pydata/xarray/pull/3713/files

# matches numpy.core.multiarray.normalize_axis_index
# duplicated here because the NumPy function is not a public API
ndim = np.ndim(data)
if not -ndim <= axis < ndim:
raise np.AxisError(f"axis {axis!r} out of bounds [-{ndim}, {ndim})")
if axis < 0:
axis += ndim
return axis


def _select_along_axis(values, idx, axis):
other_ind = np.ix_(*[np.arange(s) for s in idx.shape])
sl = other_ind[:axis] + (idx,) + other_ind[axis:]
Expand Down
12 changes: 9 additions & 3 deletions xarray/core/variable.py
Original file line number Diff line number Diff line change
Expand Up @@ -1774,14 +1774,20 @@ def quantile(

from .computation import apply_ufunc

_quantile_func = np.nanquantile if skipna else np.quantile
# TODO: switch to nanquantile/quantile once numpy >= 1.15.0 is the
# minimum requirement
_percentile_func = np.nanpercentile if skipna else np.percentile

if keep_attrs is None:
keep_attrs = _get_keep_attrs(default=False)

scalar = utils.is_scalar(q)
q = np.atleast_1d(np.asarray(q, dtype=np.float64))

# TODO: remove once numpy >= 1.15.0 is the minimum requirement
if np.count_nonzero(q < 0.0) or np.count_nonzero(q > 1.0):
raise ValueError("Quantiles must be in the range [0, 1]")

if dim is None:
dim = self.dims

Expand All @@ -1790,7 +1796,7 @@ def quantile(

def _wrapper(npa, **kwargs):
# move quantile axis to end. required for apply_ufunc
return np.moveaxis(_quantile_func(npa, **kwargs), 0, -1)
return np.moveaxis(_percentile_func(npa, **kwargs), 0, -1)

axis = np.arange(-1, -1 * len(dim) - 1, -1)
result = apply_ufunc(
Expand All @@ -1802,7 +1808,7 @@ def _wrapper(npa, **kwargs):
output_dtypes=[np.float64],
output_sizes={"quantile": len(q)},
dask="parallelized",
kwargs={"q": q, "axis": axis, "interpolation": interpolation},
kwargs={"q": q * 100, "axis": axis, "interpolation": interpolation},
)

# for backward compatibility
Expand Down