aquacropos / aquacrop

AquaCrop-OSPy: Python implementation of AquaCrop-OS
https://aquacropos.github.io/aquacrop/
Apache License 2.0
102 stars 73 forks source link

change setup.py to install pandas version that is not above 1.2.5 to avoid type error #15

Closed rbavery closed 2 years ago

rbavery commented 2 years ago

This addresses an error when running the first notebook example locally from a fresh pip install aquacrop. I moved the requirements definition to setup.py so that the more specific requirements don't need to be parsed from settings.ini.

The error is below:

→ /home/rave/miniconda3/envs/actest/bin/python /home/rave/aquacropapp/aquacropapp/model_example.py
Traceback (most recent call last):
  File "/home/rave/aquacropapp/aquacropapp/model_example.py", line 153, in <module>
    gw_model.initialize()
  File "/home/rave/aquacrop/aquacrop/core.py", line 144, in initialize
    self.ParamStruct = read_groundwater_table(self.ParamStruct,self.Groundwater,self.ClockStruct)
  File "/home/rave/aquacrop/aquacrop/initialize.py", line 419, in read_groundwater_table
    zGW = df.reindex(ClockStruct.TimeSpan,
  File "/home/rave/miniconda3/envs/actest/lib/python3.9/site-packages/pandas/util/_decorators.py", line 324, in wrapper
    return func(*args, **kwargs)
  File "/home/rave/miniconda3/envs/actest/lib/python3.9/site-packages/pandas/core/frame.py", line 4767, in reindex
    return super().reindex(**kwargs)
  File "/home/rave/miniconda3/envs/actest/lib/python3.9/site-packages/pandas/core/generic.py", line 4818, in reindex
    return self._reindex_axes(
  File "/home/rave/miniconda3/envs/actest/lib/python3.9/site-packages/pandas/core/frame.py", line 4592, in _reindex_axes
    frame = frame._reindex_index(
  File "/home/rave/miniconda3/envs/actest/lib/python3.9/site-packages/pandas/core/frame.py", line 4611, in _reindex_index
    return self._reindex_with_indexers(
  File "/home/rave/miniconda3/envs/actest/lib/python3.9/site-packages/pandas/core/generic.py", line 4883, in _reindex_with_indexers
    new_data = new_data.reindex_indexer(
  File "/home/rave/miniconda3/envs/actest/lib/python3.9/site-packages/pandas/core/internals/managers.py", line 680, in reindex_indexer
    new_blocks = [
  File "/home/rave/miniconda3/envs/actest/lib/python3.9/site-packages/pandas/core/internals/managers.py", line 681, in <listcomp>
    blk.take_nd(
  File "/home/rave/miniconda3/envs/actest/lib/python3.9/site-packages/pandas/core/internals/blocks.py", line 1145, in take_nd
    new_values = algos.take_nd(
  File "/home/rave/miniconda3/envs/actest/lib/python3.9/site-packages/pandas/core/array_algos/take.py", line 101, in take_nd
    return arr.take(
  File "/home/rave/miniconda3/envs/actest/lib/python3.9/site-packages/pandas/core/arrays/_mixins.py", line 97, in take
    fill_value = self._validate_scalar(fill_value)
  File "/home/rave/miniconda3/envs/actest/lib/python3.9/site-packages/pandas/core/arrays/datetimelike.py", line 645, in _validate_scalar
    raise TypeError(msg)
TypeError: value should be a 'Timestamp' or 'NaT'. Got 'int' instead.
thomasdkelly commented 2 years ago

Thanks Ryan,

Are these the right version requirements we want? I perform the checks above and it fails with this error:

ERROR: No matching distribution found for numpy>=1.22.0

Is requiring the latest version of numpy needed?

Also you set the numba requirement to numba==0.53.0 when the issue you pointed to says to use numba==0.55.0 https://github.com/numba/numba/issues/7339#issuecomment-997869862

thomasdkelly commented 2 years ago

Are these the right version requirements we want? I perform the checks above and it fails with this error:

ERROR: No matching distribution found for numpy>=1.22.0

Okay the issue was the github action required us to install python==3.6 which is why it couldnt find numpy=1.22.0. Also changed some other software versions to get the tests to pass: pathlib requires python<3.10 numba==0.55 requires numpy<1.22

Are you happy with these changes @rbavery ?

The pathlib library only gets used once to access a parent directory (in initialize.py) so will open a seperate pull request to change the code so we can drop that dependency.

rbavery commented 2 years ago

@thomasdkelly ah, thanks a bunch for catching these issues. This looks good to merge.