-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Add a type guard for intX
#4569
Conversation
@lucianopaz pymc3/tests/test_model_helpers.py::TestHelperFunc::test_pandas_to_array this test does not pass. I've changed intX to be tolerant to int dtypes but it complains about new behavior: not downcasting int32 to int16. What is treated as the expected behavior here? |
also, @brandonwillard, do we really need downcasting there? |
Sorry for taking so long @ferrine. I honestly don’t know what’s best here. Given that you’ve changed the intX behaviour it would make sense to also change the test function to adapt to the new behaviour. I imagine that you would have to change the if statement to make it more lenient with ints. |
I'll fix the test soon as it seems we all agree on the change |
I've fixed the failings tests, feel free to review and merge |
@lucianopaz or @brandonwillard can you please have a look at this? |
@ferrine not converting already int data sounds like a good idea. And your solution is simple yet elegant. |
What about pushing this to the V4? |
Yeah, we should. |
* add type guard for inX * fix test for pandas * fix posterior test, ints passed for float data Closes pymc-devs#4279
Integer dtypes are now not downcasted in intX
In discrete distributions there is a usage of
intX
that enforces unreasonable limitations in UX (see Do not force lower precision in intX #4553)I did not yet create a unit test covering this specific usecase
Ran
black
on itNot required
Note: Questions before we go ahead, the alternative would be leave the function as is and covert parameters in Discrete distributions