Skip to content

update and sync dependencies#144

Merged
FabianHofmann merged 3 commits into
masterfrom
fix-dependencies
Jun 8, 2021
Merged

update and sync dependencies#144
FabianHofmann merged 3 commits into
masterfrom
fix-dependencies

Conversation

@fneum

@fneum fneum commented May 26, 2021

Copy link
Copy Markdown
Member

Closes # (if applicable).

Change proposed in this Pull Request

Update, correct and synchronise dependencies.

Description

  • minimum pandas version seems to be pandas=0.25 which is the minimum of xarray>=0.16.2 (which is now also set consistently).
  • dask memory regression not yet resolved Regression: poor memory management in threads and sync schedulers dask/dask#7583 but newer versions released, resulting in dask<2021.04.0
  • tqdm is used since recently, but was not added to list of dependencies
  • environment.yaml missed progressbar and requests
  • setup.py requested Python 3.6 but newer versions also work.
  • Why was the conda-forge channel wedged between two defaults?

Motivation and Context

Some dependencies were not correctly specified and increasingly out of sync across setup.py, environment.yaml and the conda-feedstock (see conda-forge/atlite-feedstock#14)

How Has This Been Tested?

For environment.yaml:

conda env create -f environment.yaml
pip install --no-deps -e .
pytest

(another time where I fixed pandas=0.25 and python=3.6)

For setup.py:

conda env create -n atlite-pip python=3.8 pip pytest
pip install -e .
pytest

For all, I had problems with

  • test_dx_dy_preservation_era5_weird
  • test_all_non_na_era5_weird

and I could not figure out why. They just get stuck (not windows!).

I also had problems with update_feature_test:

>           ds = (cutout.data.merge(ds[missing_vars.values])
                  .assign_attrs(**non_bool_dict(cutout.data.attrs), **ds.attrs))
E           TypeError: xarray.core.common.DataWithCoords.assign_attrs() got multiple values for keyword argument 'Conventions'

atlite/data.py:159: TypeError

Any suspicions where the extra keyword argument comes from?

I did not run the SARAH tests.

I could open separate issues for these if they substantiate.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I tested my contribution locally and it seems to work fine.
  • I locally ran pytest inside the repository and no unexpected problems came up.
  • I have adjusted the docstrings in the code appropriately.
  • I have documented the effects of my code changes in the documentation doc/.
  • I have added newly introduced dependencies to environment.yaml file.
  • I have added a note to release notes doc/release_notes.rst.

@euronion

Copy link
Copy Markdown
Collaborator
  • pytest is missing from setup.py.
  • (windows problem) Installation with pip on windows fails due to GDAL (rasterio dep), conda works fine (Probably not a new issue, windows and gdal have always been a troublesome combination)
  • (windows problem) multiple tests on windows with pytest failed due to problems in the installation of rtree (geopandas dep).

There is some discussion over in the geopandas and rtree community over this with many different "solutions" being presented but nothing I tried solved the problem neatly.
I suggest we accept this problem on windows for now and hope it will be resolved soon; it does not seem to be a problem related to the depdency specification but upstream issues.

Regarding the tests

and I could not figure out why. They just get stuck (not windows!).

Yes, it is a familiar problem. Restarting always did the trick in the past. We've speculated it is linked to the combination of multithreading + cdsapi. But haven't been able to track down the problem.

On the problems in update_feature_test:
This might be a real bug. I'll create a dedicated issue. The same error is raised multiple times for the SARAH test.

I also encountered a few deprecation warning which need fixing.

Given the nature of the problems I believe we're good for merging this PR.

@FabianHofmann
FabianHofmann merged commit 06371b7 into master Jun 8, 2021
@fneum
fneum deleted the fix-dependencies branch December 9, 2021 13:49
mrmorawski pushed a commit to mrmorawski/atlite that referenced this pull request May 10, 2026
* update and sync dependencies

* env: add toolz
setup.py: add toolz + pytest

Co-authored-by: Fabian <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Regression: poor memory management in threads and sync schedulers

3 participants