wearepal / data-science-types

Mypy stubs, i.e., type information, for numpy, pandas and matplotlib
Apache License 2.0
202 stars 52 forks source link

Fix numpy arange overload #194

Closed Hvlot closed 3 years ago

Hvlot commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

Hvlot commented 3 years ago

Great, that seems to have worked!

tmke8 commented 3 years ago

Perfect! Thanks!