-
Notifications
You must be signed in to change notification settings - Fork 62
Fix pandas FutureWarning in adbscan.remap_lbls() #406
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
base: main
Are you sure you want to change the base?
Fix pandas FutureWarning in adbscan.remap_lbls() #406
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes a pandas FutureWarning in adbscan.remap_lbls() that occurs when attempting to assign NaN values to integer-typed DataFrame columns. The fix adds dtype compatibility checking before assignment to prevent the warning.
Changes:
- Modified
esda/adbscan.pyto add dtype promotion logic before assigning mapped values - Added regression test
esda/tests/test_issue_342.pyto verify the warning is resolved - Applied code formatting improvements to multiple notebook and test files
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| esda/adbscan.py | Added dtype check and promotion logic to prevent FutureWarning in remap_lbls() |
| esda/tests/test_issue_342.py | New test to verify FutureWarning is not raised |
| notebooks/*.ipynb | Code formatting improvements (spacing, quotes) |
| esda/tests/*.py | Code formatting improvements (spacing, line breaks) |
| esda/moran_local_mv.py | Code formatting improvements |
| esda/significance.py | Code formatting improvements |
| esda/crand.py | Code formatting improvements |
| esda/init.py | Code formatting improvements |
Comments suppressed due to low confidence (1)
esda/adbscan.py:340
- The dtype compatibility fix (lines 347-350) is only applied to the single-core path but not to the multi-core path. When n_jobs > 1, the same FutureWarning can occur because
_remap_n_expandreturns mapped values that may contain NaN, which are then assigned to integer-typed columns in line 340. The same dtype checking and promotion logic should be applied before or after line 340 to ensure consistency across both execution paths.
remapped = pool.map(_remap_n_expand, to_loop_over)
remapped_df = pandas.concat(remapped, axis=1)
remapped_solus.loc[:, s_ids] = remapped_df
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| with warnings.catch_warnings(record=True) as w: | ||
| warnings.filterwarnings("always", category=FutureWarning) | ||
| lbls = adbscan.remap_lbls(solus, db, n_jobs=1) |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test only validates the single-core path (n_jobs=1) but does not test the multi-core path (n_jobs=-1 or n_jobs > 1). Since the fix is incomplete and doesn't cover the parallel execution path, a test case for the multi-core scenario should be added to ensure both code paths work correctly.
esda/tests/test_moran_local_mv.py
Outdated
| y,X,df = data | ||
| m = MoranLocalPartial(permutations=1).fit(X,y,graph) | ||
| y, X, df = data | ||
| m = MoranLocalPartial(permutations=1).fit(X, y, graph) |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Variable m is not used.
| m = MoranLocalPartial(permutations=1).fit(X, y, graph) | |
| MoranLocalPartial(permutations=1).fit(X, y, graph) |
esda/tests/test_moran_local_mv.py
Outdated
| a = MoranLocalConditional(permutations=1).fit(X,y,graph) | ||
| #done, just check if it runs | ||
| y, X, df = data | ||
| a = MoranLocalConditional(permutations=1).fit(X, y, graph) |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Variable a is not used.
| a = MoranLocalConditional(permutations=1).fit(X, y, graph) | |
| MoranLocalConditional(permutations=1).fit(X, y, graph) |
esda/tests/test_moran_local_mv.py
Outdated
| a = MoranLocalConditional(permutations=0, transformer=TheilSenRegressor).fit(X,y,graph) | ||
| # done, should just complete No newline at end of file | ||
| y, X, df = data | ||
| a = MoranLocalConditional(permutations=0, transformer=TheilSenRegressor).fit( |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Variable a is not used.
| a = MoranLocalConditional(permutations=0, transformer=TheilSenRegressor).fit( | |
| MoranLocalConditional(permutations=0, transformer=TheilSenRegressor).fit( |
| @@ -0,0 +1,42 @@ | |||
| import warnings | |||
|
|
|||
| import numpy as np | |||
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'np' is not used.
| import numpy as np |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #406 +/- ##
=====================================
Coverage 82.7% 82.7%
=====================================
Files 27 27
Lines 3829 3832 +3
=====================================
+ Hits 3166 3169 +3
Misses 663 663
🚀 New features to boost your workflow:
|
|
|
Okay @jGaboardi |
0431a5b to
5b8ab98
Compare
fixes #342.
Solution
Add dtype compatibility check before assignment:
dtype.kind == 'i'), promote to float64Code changes (3 lines added):
This approach:
.fillna()downstream)Testing
New test added:
esda/tests/test_issue_342.pyFutureWarningis raised after the fixRegression testing:
test_adbscan.pytests pass