Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
np.concatenate cleanups.#15679
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
np.concatenate cleanups.#15679
Conversation
Replace np.concatenate by iterable unpacking when the arrays are short(so performance doesn't matter) and legibility is improved, or bynp.roll(), or by np.append.
| n =hour_limits.searchsorted(dv) | ||
| n =np.searchsorted(hour_limits,dv) | ||
| step = hour_steps[n] |
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.
How does this work?n is an array still, buthour_steps is a list now, and I can't index a list with an array.
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.
dv is a scalar (this is called by select_step, where it is immediately preceded byif dv > 1 / threshold_factor -- semantically, it is the distance between the axes bounds) so n is a scalar as well and the indexing is fine.
I guesstechnically it is an API break in that select_step_degreecould have been called directly by someone passing dv as an array, but I don't think this is a real issue (and is clearly not how select_step_degree was designed to do).
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.
s/preceded/succeeded/, but I see what you mean.
Replace np.concatenate by iterable unpacking when the arrays are short
(so performance doesn't matter) and legibility is improved, or by
np.roll(), or by np.append.
PR Summary
PR Checklist