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
Fix np.where logic in table.join #9313
Conversation
astropy/table/operations.py
Outdated
if np.any(right_mask): | ||
out[out_name][right_mask] = right[right_name].take(right_out) | ||
out[out_name] = left[left_name].take(left_out) |
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.
Why this change? And should this not be an in-place change at least? (with [:]
)
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.
indeed, it should use [:]
. As for the rest of the changes, I did the fix to follow the very same logic as the np.where
above (the issue with that it doesn't understand Quantities for np <1.17 and astropy <4.0):
out[out_name][:] = np.where(right_mask,
left[left_name].take(left_out),
right[right_name].take(right_out))
ffb02e8
to
58b235b
Compare
(a rebase has solved the weird appveyor issue, so if we don't see it popping up elsewhere we are good) |
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.
This looks good with @astrofrog's comment addressed, so approving.
Fix np.where logic in table.join
Fix np.where logic in table.join
Fix np.where logic in table.join
Fix np.where logic in table.join
This regression was unearthed while doing #9287, and thus be opened as a separate PR to be able to backport the fix.