Skip to content
This repository has been archived by the owner on Feb 17, 2021. It is now read-only.

Fix numpy arange overload #194

Merged
merged 4 commits into from Dec 1, 2020
Merged

Fix numpy arange overload #194

merged 4 commits into from Dec 1, 2020

Conversation

Hvlot
Copy link
Contributor

@Hvlot Hvlot commented Nov 30, 2020

Change order of start/stop to comply with numpy documentation (https://numpy.org/doc/stable/reference/generated/numpy.arange.html) and change data types to float.

This is my first ever PR on github for a public repository, so please be gentle. If I need to clear anything up, please let me know.

@tmke8
Copy link
Member

tmke8 commented Nov 30, 2020

Hi! Thank you for your PR!

So, looking at the documentation, it seems that if any of start, stop or step is a float, then the return type is an array of float64. Is that right?

If this is the case, then ideally we would add overloads of np.arange which change the output type depending on the input type. It seems you already tried something like this? What was the problem?

@Hvlot
Copy link
Contributor Author

Hvlot commented Nov 30, 2020

Hi! Thank you for your PR!

So, looking at the documentation, it seems that if any of start, stop or step is a float, then the return type is an array of float64. Is that right?

If this is the case, then ideally we would add overloads of np.arange which change the output type depending on the input type. It seems you already tried something like this? What was the problem?

I tested it to make sure and indeed, when any of start, stop or step is a float, numpy will return an ndarray with dtype of float64.

I tried to make a second overload with the special case where all inputs are ints, however the test failed because Overloaded function signature 2 will never be matched: signature 1's parameter type(s) are the same or broader, which is not allowed apparently. I guess it makes sense, but then I don't know how to handle the special case where all inputs are ints, in which case numpy returns an ndarray with int32 or int64, depending on the size of the input integers.

@tmke8
Copy link
Member

tmke8 commented Dec 1, 2020

Ah, try reversing the order of the overloads. So, put first the overload with all ints and then the overload with all floats.

@Hvlot
Copy link
Contributor Author

Hvlot commented Dec 1, 2020

Great, that seems to have worked!

@tmke8
Copy link
Member

tmke8 commented Dec 1, 2020

Perfect! Thanks!

@tmke8 tmke8 merged commit de94f0d into wearepal:master Dec 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants