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

Replace pandas.df.append with safe_concat #259

Merged
merged 7 commits into from
Nov 15, 2023
Merged

Conversation

alessandrofelder
Copy link
Member

Description

What is this PR

  • Bug fix
  • Addition of a new feature
  • Other

Why is this PR needed?
Newer versions of pandas break the analysis code, due to the deprecation and removal of pandas.Dataframe.append.

What does this PR do?

  • Replaces the use of pandas.Dataframe.append with brainglobe_utils.pandas.safe_concat in analyse.get_region_totals()
  • Adds a regression test to check that the result is still the same as with pandas 1.5.3, ie before deprecation).

References

Closes #254

How has this PR been tested?

  • Regression data generated with pandas 1.5.3 and test written.

Is this a breaking change?

Nope

Does this PR require an update to the documentation?

Nope

Checklist:

  • The code has been tested locally
  • Tests have been added to cover all new functionality (unit & integration)
  • The documentation has been updated to reflect any changes
  • The code has been formatted with pre-commit

alessandrofelder and others added 7 commits November 13, 2023 16:24
reproduces #254
fails as expected with pandas 2.1.3
passes with pandas 1.5.3
a dummy volumes.csv created by cutting down version from registration_output data
avoid using hard-coded paths and writing tmp data to codebase
@alessandrofelder alessandrofelder self-assigned this Nov 14, 2023
@alessandrofelder alessandrofelder added bug Something isn't working and removed bug Something isn't working labels Nov 14, 2023
Copy link

codecov bot commented Nov 14, 2023

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (0e3301c) 23.31% compared to head (636df4b) 24.61%.

Files Patch % Lines
cellfinder/analyse/analyse.py 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #259      +/-   ##
==========================================
+ Coverage   23.31%   24.61%   +1.30%     
==========================================
  Files          22       23       +1     
  Lines        1214     1235      +21     
==========================================
+ Hits          283      304      +21     
  Misses        931      931              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@alessandrofelder alessandrofelder marked this pull request as ready for review November 14, 2023 18:33
@willGraham01 willGraham01 merged commit e5f3b6c into main Nov 15, 2023
13 checks passed
@willGraham01 willGraham01 deleted the fix-pandas-deprecation branch November 15, 2023 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Deprecation of pandas.DataFrame.append() causing cellfinder to crash
2 participants