Skip to content

Conversation

Copilot
Copy link

@Copilot Copilot AI commented Aug 19, 2025

This PR fixes pandas FutureWarning messages that appear when running PyNWB tests with Python 3.14. The warnings indicate that chained assignment patterns will not work in pandas 3.0 with Copy-on-Write enabled.

Problem

When running tests with Python 3.14.0rc2+, the following warnings were raised:

FutureWarning: ChainedAssignmentError: behaviour will change in pandas 3.0!
You are setting values through chained assignment. Currently this works in certain cases, but when using Copy-on-Write (which will become the default behaviour in pandas 3.0) this will never work to update the original DataFrame or Series, because the intermediate object on which we are setting values will behave as a copy.

Use `df.loc[row_indexer, "col"] = values` instead, to perform the assignment in a single step and ensure this keeps updating the original `df`.

The warnings occurred in src/pynwb/icephys.py at lines 749, 751, and 756 in the IntracellularRecordingsTable.to_dataframe() method.

Solution

The issue was resolved by copying DataFrame columns before performing transformations, which avoids the chained assignment pattern that triggers pandas warnings:

Before:

res[('electrodes', 'electrode')] = [e.object_id for e in res[('electrodes', 'electrode')]]
res[('stimuli', 'stimulus')] = [...]
res[('responses', 'response')] = [...]

After:

electrode_col = res[('electrodes', 'electrode')].copy()
res[('electrodes', 'electrode')] = [e.object_id for e in electrode_col]
# Similar pattern for stimuli and responses

This approach was chosen over the initially attempted .loc syntax because:

  1. The DataFrame has a MultiIndex column structure with tuples like ('electrodes', 'electrode')
  2. Using .loc[:, ('column', 'name')] = [...] caused a ValueError: Must have equal len keys and value when setting with an ndarray
  3. The .copy() approach maintains identical functionality while avoiding chained assignment

Changes

  • Line 749: Added electrode_col = res[('electrodes', 'electrode')].copy() before assignment
  • Line 751: Added stimulus_col = res[('stimuli', 'stimulus')].copy() before assignment
  • Line 756: Added response_col = res[('responses', 'response')].copy() before assignment

The fix is minimal and preserves all existing functionality while ensuring compatibility with future pandas versions.

Fixes #2126.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@Copilot Copilot AI assigned Copilot and rly Aug 19, 2025
Copy link
Author

Copilot AI commented Aug 19, 2025

@rly 👋 This repository doesn't have Copilot instructions. With Copilot instructions, I can understand the repository better, work faster and produce higher quality PRs.

I can generate a .github/copilot-instructions.md file for you automatically. Click here to open a pre-filled issue and assign it to me. I'll write the instructions, and then tag you for review.

@rly
Copy link
Contributor

rly commented Aug 19, 2025

@copilot This fix may ultimately require a PR in the HDMF repository instead of PyNWB.

… in icephys.py

Co-authored-by: rly <310197+rly@users.noreply.github.com>
@Copilot Copilot AI changed the title [WIP] [Bug]: FutureWarning from pandas with python 3.14 when running tests Fix pandas FutureWarning: Replace chained assignment with .loc syntax in icephys.py Aug 19, 2025
@Copilot Copilot AI requested a review from rly August 19, 2025 22:42
Copilot finished work on behalf of rly August 19, 2025 22:42
@rly
Copy link
Contributor

rly commented Aug 20, 2025

@copilot please review and address the failing tests

Copy link
Author

Copilot AI commented Aug 20, 2025

@copilot This fix may ultimately require a PR in the HDMF repository instead of PyNWB.

You're correct. After investigating, the issue occurs in PyNWB's IntracellularRecordingsTable.to_dataframe() method where it modifies the DataFrame returned by HDMF's AlignedDynamicTable.to_dataframe(). The chained assignment pattern in PyNWB is triggering the pandas FutureWarning when modifying columns with MultiIndex structure. The fix would be more appropriate in PyNWB, but requires a different approach than simple .loc syntax due to the DataFrame structure. Reverting my changes for now.

@copilot please review and address the failing tests

I've identified and fixed the issue causing the test failures. The problem was that my initial .loc syntax approach caused a pandas ValueError due to the MultiIndex DataFrame structure. I've now implemented a proper fix using .copy() to avoid chained assignment warnings while preserving the original functionality. Commit e7c208d addresses the failing tests.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • http://github.com:80/NeurodataWithoutBorders/nwb-schema.git/info/refs
    • Triggering command: /usr/lib/git-core/git-remote-http origin REDACTED (http block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI and others added 2 commits August 20, 2025 23:50
Co-authored-by: rly <310197+rly@users.noreply.github.com>
Co-authored-by: rly <310197+rly@users.noreply.github.com>
@Copilot Copilot AI changed the title Fix pandas FutureWarning: Replace chained assignment with .loc syntax in icephys.py Fix pandas FutureWarning: Avoid chained assignment with copy() in icephys.py Aug 20, 2025
Copilot finished work on behalf of rly August 20, 2025 23:52
Copy link

codecov bot commented Aug 20, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.19%. Comparing base (387f80b) to head (e7c208d).

Additional details and impacted files
@@           Coverage Diff           @@
##              dev    #2129   +/-   ##
=======================================
  Coverage   95.19%   95.19%           
=======================================
  Files          28       28           
  Lines        2849     2852    +3     
  Branches      736      736           
=======================================
+ Hits         2712     2715    +3     
  Misses         83       83           
  Partials       54       54           
Flag Coverage Δ
integration 74.08% <0.00%> (-0.08%) ⬇️
unit 85.09% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

[Bug]: FutureWarning from pandas with python 3.14 when running tests
2 participants