AllenInstitute / AllenSDK

code for reading and processing Allen Institute for Brain Science data
https://allensdk.readthedocs.io/en/latest/
Other
343 stars 149 forks source link

Resolve dependencies refresh #2710

Closed mikejhuang closed 1 year ago

mikejhuang commented 1 year ago

Include Python 3.11 compatibility, unpeg all dependencies and fix breaking changes from updated packages or downgrade packages.

Steps

  1. Use poetry to setup pyproject.toml file with python = ">=3.8,<3.12"
  2. Create a copy of requirements.txt without explicit versions at poetry_py38-311.txt
  3. poetry add `cat poetry_py38-311.txt`
  4. poetry does not resolve python version requirements and this needs to be adjusted manually
  5. xarray, scipy, and numpy versions adjusted to meet python version requirements
  6. poetry add `cat poetry_py38-311.txt` completed successfully and created solution of valid dependency ranges in pyproject.toml
  7. All dependencies are installed, poetry.lock file stores the actual installed versions for reproducibility
  8. Run tests by creating virtualenv with 3.8, 3.9, 3.10, 3.11
  9. For failing tests, either fix code or revert package version Final explicitly defined requirements

Fixes for compatibility

  1. pandas 1.1.5 -> 2.0.3
    • Int64Index deprecated https://github.com/pandas-dev/pandas/issues/42717
    • pd.Series.str.replace must specify regex=True if regex is used.
    • pd.Series.reindex with a MultiIndex can no longer take in single integer Index as an input
    • check_less_precise is a deprecated parameter in pd.testing.assert_series_equal. This is replaced with r_tol and a_tol
  2. numpy 1.23.5
    • np.bool, np.float, np.int are deprecated. Use 'bool', 'float', or 'int' instead.

Reverting versions instead of fixing

  1. hdmf=<3.4.7
  2. pandas<2.0.0
    • padas==2.0.3: VisualBehaviorProjectCache.get_ophys_session_table( index_column="ophys_experiment_id").index is type 'object', expected int64 All the values in this index are int64 so I'm not sure why it was loaded as an object type.
review-notebook-app[bot] commented 1 year ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

mikejhuang commented 1 year ago

I intended to consolidate the dependencies in pyproject.toml and remove the requirements.txt after I understood it better when reading about it more and discussing it with the rest of the team.

I renamed this PR from "Poetry dependency manager" to "Resolve dependencies refresh" since the Poetry aspect was a tool used to build the pyproject.toml but is not a requirement for this package. I believe it would be up to each developer to use Poetry if they believe it adds value to the way they manage dependencies. You can still manually edit the versions in the pyproject.toml file and use pip if you don't want Poetry to define them for you.

aamster commented 1 year ago

I believe it would be up to each developer to use Poetry if they believe it adds value to the way they manage dependencies.

I am not going to adopt poetry right now. That leaves Chris. @morriscb will you be adopting poetry for development? If the majority of developers are not going to adopt it, then let's remove pyproject.toml, poetry.lock, etc from this PR.

morriscb commented 1 year ago

I believe it would be up to each developer to use Poetry if they believe it adds value to the way they manage dependencies.

I am not going to adopt poetry right now. That leaves Chris. @morriscb will you be adopting poetry for development? If the majority of developers are not going to adopt it, then let's remove pyproject.toml, poetry.lock, etc from this PR.

I do not plan to adopt poetry at this time, no.

mikejhuang commented 1 year ago

This should be trageted onto branch rc/2.16.0.

I recall agreeing that it should be targeted to main to expedite this so that users of SWDB would have these changes.

morriscb commented 1 year ago

The fix for SWDB has already been deployed and is working. That was the tickets we were referring to, not tihs one.

mikejhuang commented 1 year ago

The fix for SWDB has already been deployed and is working. That was the tickets we were referring to, not tihs one.

I recall it was also for this one since it would reduce the compatibility issues users might run into during that workshop.