python-mip icon indicating copy to clipboard operation
python-mip copied to clipboard

"numbers.Real" makes it really hard to work with typechecking

Open joaomanke opened this issue 3 years ago • 6 comments

Describe the bug When working with mypy arguments of type numbers.Real cannot accept regular float or int values. Making working with them very unproductive due to having to typing.cast(numbers.Real, value) all over the code.

To Reproduce Run mypy on this file for example:

import numbers
import typing

import mip

model = mip.Model("Model")

x = 1
model.add_var("var1", lb=x) # "Literal[1]" is incompatible with "Real"

y = 1.0
model.add_var("var2", lb=y) # "float" is incompatible with "Real"

z: numbers.Real = 5.6 # "float" is incompatible with "Real"
model.add_var("var3", lb=z)

z = typing.cast(numbers.Real, 7.8) # have to do this everytime
model.add_var("var4", lb=z) # no problem

Edit 1: Additional steps are needed to generate the stub for the library.

export MYPYPATH=./out

stubgen -p mip
mypy main.py

Expected behavior I understan reasoning for using number.Real however even the PEP 484 suggests using float instead:

Rather than requiring that users write import numbers and then use numbers.Float etc., this PEP proposes a straightforward shortcut that is almost as effective: when an argument is annotated as having type float, an argument of type int is acceptable; similar, for an argument annotated as having type complex, arguments of type float or int are acceptable. This does not handle classes implementing the corresponding ABCs or the fractions.Fraction class, but we believe those use cases are exceedingly rare.

Desktop (please complete the following information):

  • Operating System, version: Windows 10 64bit
  • Python version: 3.9
  • Python-MIP version (we recommend you to test with the latest version): 1.13

Additional context I know this isn't a proper bug, however it has been very frustrating to work with.

joaomanke avatar Jan 21 '21 16:01 joaomanke

Not sure I can reproduce this. Under py3.7/osx/mip1.13.0 Running mypy main.py I get the output

numbers_real.py:4: error: Skipping analyzing 'mip': found module but no type hints or library stubs
numbers_real.py:4: note: See https://mypy.readthedocs.io/en/latest/running_mypy.html#missing-imports
numbers_real.py:14: error: Incompatible types in assignment (expression has type "float", variable has type "Real")
Found 2 errors in 1 file (checked 1 source file)

Very curious about this though. IIRC the rationale behind using numbers.Real was to allow numpy types to be used.

--edit

https://github.com/coin-or/python-mip/commit/f65eb9512a8c63bb2507c40d1b4587d5efe17f99 int and float changed to numbers.Real with commit message "Improving numpy compatibility" - unfortunately no PR or issues associated with it that I can find.

jurasofish avatar Jan 21 '21 22:01 jurasofish

@jurasofish Indeed I forgot some steps in the reproduction, I'll edit it in.

Basically even though mip is typed it doen't have a type stub which we can generate with mypy as well:

export MYPYPATH=./out
stubgen -p mip
mypy main.py

But you can see in the third line of your execution:

numbers_real.py:14: error: Incompatible types in assignment (expression has type "float", variable has type "Real")

That indeed the types don't match, the other ones didn't appear because without the stub all of mip is treated as typing.Any.

Edit: Also I don't really use numpy, but if mip is intended to be used mostly in conjunction with it I guess maybe that's the way it should be.

joaomanke avatar Jan 22 '21 01:01 joaomanke

Yeah looks like this is big point of contention actually https://github.com/python/mypy/issues/3186

Perhaps we should use something like

mip.Real = Union[float, int, np.float]

Also would be good to add this to CI

jurasofish avatar Jan 25 '21 22:01 jurasofish

I don't know if you saw, but I pulled out a lot of my own number typing work-arounds into their own package called numerary. If this journey lines up with your own, numerary could be an intermediary solution until python/mypy#3186 is fixed. (Alternatively, the techniques used therein might provide inspiration if you can't take a dependency.) Happy to consult here, if helpful. Apologies if this is a distraction.

posita avatar Dec 15 '21 15:12 posita

I am going to add mypy to CI. See #270. There I started to refactor the numbers.Real for typing to make it mypy compatible.

sebheger avatar May 07 '22 20:05 sebheger

The mip.Real solution probably makes the most sense. Fulfilling the spirit of PEP 3141 that mypy is failing to observe.

AdeelK93 avatar Sep 23 '23 18:09 AdeelK93