-
Notifications
You must be signed in to change notification settings - Fork 1
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
Column locator sequence input #415
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.
Thanks for trying to tackle this.
The wrong typing most likely comes from my test supporting lists instead of scalars to speed processing up in #370, but as I was not able to find a meaningful speed up, I reverted the changes in #393.
I would suggest to solve #407 by changing the typing to support only scalars.
Co-authored-by: Hagen Wierstorf <hwierstorf@audeering.com>
Co-authored-by: Hagen Wierstorf <hwierstorf@audeering.com>
Co-authored-by: Hagen Wierstorf <hwierstorf@audeering.com>
8063281
to
80289a1
Compare
Thanks for updating the code. Can you also update the description of the pull request, so it's easy to see later on, what was added here. |
BTW, I checked how we avoid calling Line 109 in 075b279
|
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.
As stated in #415 (comment) this is ready to merge, but would be nice to first update the description.
I have updated the PR description above. Merging. |
Goal
close #407: Typing provided squence type input to
Dependencies._column_loc
but it wasn't implemented.Solution
There was a mismatch between the type hints suggesting that it is possible to use sequence input and output.
We have chosen to update the type hints as the suggested vectorized implenentation did not provide large benefits.
Therefore this corrects the type hints allowing only scalar in- and putput.