Skip to content

remove more of the mypy ignore list #830

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

Merged
merged 9 commits into from
May 20, 2025
Merged

remove more of the mypy ignore list #830

merged 9 commits into from
May 20, 2025

Conversation

frrad
Copy link
Contributor

@frrad frrad commented Apr 9, 2025

This removes the last of the "real" exceptions from pyproject.toml, adding type: ignore as needed.

@frrad frrad force-pushed the type_more branch 2 times, most recently from 52250fc to 16fe242 Compare April 9, 2025 05:06
@@ -163,10 +163,9 @@ def __init__(

# set the output of gurobi
if not self.msg:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is unrelated, just saw it as I was reading the code.

@@ -14,6 +14,15 @@ def check_dummy_env():
pass


def is_single_use_license() -> bool:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the SkipTest decorators below failed to type check. I was deciding between this and just unilateral skip with @unittest.skip. Happy to go the other way if you want.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really understand the difference between unittest.SkipTest and unittest.skip. Maybe @torressa knows ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think they can be changed to unittest.skip. Not sure about the SkipTest decorator is working as expected anymore. In any case, we really want to skip these as they are only relevant when using a specific license (single use).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to check for the single use license inside the is_single_use_license function and return true/false?

Copy link
Contributor

@torressa torressa Apr 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With gurobipy one could do:

def is_single_use_license() -> bool:
    try:
        with gp.Env() as env1:
            with gp.Env() as env2:
                pass
    except gp.GurobiError as ge:
        if ge.errno == gp.GRB.Error.NO_LICENSE:
            print("single use license")
            print(f"Error code {ge.errno}: {ge}")
            return True
    return False

This is enough to trigger an error in a single-use case.

@@ -1574,7 +1576,7 @@ def test_constraint_add(self):
self.assertEqual(str(c1_variable), str(2 * x + y <= 5))
self.assertEqual(repr(c1_variable), repr(2 * x + y <= 5))

expr: LpAffineExpression = x + 1
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mypy complains about these type hints since these variables are already defined.

@frrad frrad marked this pull request as ready for review April 9, 2025 06:06
@frrad
Copy link
Contributor Author

frrad commented Apr 9, 2025

I have put in the implementation supplied by @torressa. Thanks!

On my machine these tests are still skipped. If you have a single-use license environment to play with it might be good to verify that they are run and pass.

pytest -rs pulp/tests/test_gurobipy_env.py
================================================== test session starts ===================================================
platform linux -- Python 3.10.14, pytest-8.3.5, pluggy-1.5.0
rootdir: /home/frederick/Projects/pulp
configfile: pyproject.toml
plugins: darkgraylib-1.2.1
collected 7 items                                                                                                        

pulp/tests/test_gurobipy_env.py s.ss...                                                                            [100%]

================================================ short test summary info =================================================
SKIPPED [1] pulp/tests/test_gurobipy_env.py:83: this test is only expected to pass with a single-use license
SKIPPED [1] pulp/tests/test_gurobipy_env.py:56: this test is only expected to pass with a single-use license
SKIPPED [1] pulp/tests/test_gurobipy_env.py:123: this test is only expected to pass with a single-use license
============================================== 4 passed, 3 skipped in 0.23s ==============================================

@pchtsp
Copy link
Collaborator

pchtsp commented May 19, 2025

it seems to still fail with the current changes, @frrad could you take a look? thanks!

@frrad
Copy link
Contributor Author

frrad commented May 19, 2025

It was working! I wonder what changed 🤔 will take a look.

@frrad
Copy link
Contributor Author

frrad commented May 19, 2025

Fixed! Looks like some minor conflict with #831

@pchtsp pchtsp merged commit 5d74502 into coin-or:master May 20, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants