Skip to content

Fix pandas 1.3/1.4 support #148

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

Merged
merged 5 commits into from
Jul 10, 2021
Merged

Conversation

xiki-tempula
Copy link
Collaborator

@xiki-tempula xiki-tempula commented Jul 9, 2021

Fix #147

Due to pandas-dev/pandas#42433, this will need some fix from upstream.
I have done a patch to fix the problem.

@codecov
Copy link

codecov bot commented Jul 9, 2021

Codecov Report

Merging #148 (566f6ae) into master (dce2893) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #148   +/-   ##
=======================================
  Coverage   97.70%   97.70%           
=======================================
  Files          20       20           
  Lines        1091     1091           
  Branches      230      230           
=======================================
  Hits         1066     1066           
  Misses          5        5           
  Partials       20       20           
Impacted Files Coverage Δ
src/alchemlyb/estimators/ti_.py 100.00% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dce2893...566f6ae. Read the comment docs.

@xiki-tempula xiki-tempula requested a review from orbeckst July 9, 2021 15:47
Copy link
Member

@orbeckst orbeckst left a comment

Choose a reason for hiding this comment

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

Good for a quick fix. Eventually we want to revert the fix and just exclude buggy versions of pandas in setup.py.

I see that pandas has already a bug fix PR pandas-dev/pandas#42469 and the next patch release will likely have it fixed.

Please add comments to the hot fix lines and also add a comment to setup.py for which versions of pandas we'll eventually exclude. Once that's done, merge, please. Thank you!

@@ -61,7 +61,7 @@ def fit(self, dHdl):
l_types = dHdl.index.names[1:]

# obtain vector of delta lambdas between each state
dl = means.reset_index()[means.index.names[:]].diff().iloc[1:].values
dl = means.reset_index()[list(means.index.names[:])].diff().iloc[1:].values
Copy link
Member

Choose a reason for hiding this comment

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

add a comment that we undo this fix once pandas is fixed

@@ -133,7 +133,7 @@ def separate_dhdl(self):
# get the lambda names
l_types = self.dhdl.index.names
# obtain bool of changed lambdas between each state
lambdas = self.dhdl.reset_index()[l_types]
lambdas = self.dhdl.reset_index()[list(l_types)]
Copy link
Member

Choose a reason for hiding this comment

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

add a comment that we undo this fix once pandas is fixed

@orbeckst
Copy link
Member

Note: the fix PR pandas-dev/pandas#42469 is in their 1.3.1 milestone so we will just have to exclude 1.3.0 in the install_requires list

install_requires=['numpy', 'pandas>=1.2', 'pymbar>=3.0.5,<4', 'scipy', 'scikit-learn', 'matplotlib']
as

"pandas>=1.2,!=1.3.0"

@xiki-tempula xiki-tempula merged commit 51c28a5 into alchemistry:master Jul 10, 2021
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.

tests for Python 3.7 started failing with 'TypeError: Argument 'obj' has incorrect type (expected list, got FrozenList)'
2 participants