PipProvider._known_depths sets depths to math.inf incorrectly

[notatallshaw]

Description

While investigating #10201 using the following requirements:

pytest-cov
coverage==4.5.4
boto3

I printed out the inferred_depth in the method PipProvider.get_preference and noticed that almost all the packages, other than those listed above, had their inferred_depth calculated as math.inf.

Expected behavior

The actual inferred depth should be calculated. This can be fixed by moving the line to after the try/except statement:

self._known_depths[identifier] = inferred_depth

E.g. changing the code to:

try:
    requested_order: Union[int, float] = self._user_requested[identifier]
except KeyError:
    requested_order = math.inf
    parent_depths = (
        self._known_depths[parent.name] if parent is not None else 0.0
        for _, parent in information[identifier]
    )
    inferred_depth = min(d for d in parent_depths) + 1.0
else:
    inferred_depth = 1.0

self._known_depths[identifier] = inferred_depth

pip version

21.2.4

Python version

all

OS

all

How to Reproduce

1. Run pip install/download on the above requirements text

Output

No response

Code of Conduct

• I agree to follow the PSF Code of Conduct.

[notatallshaw]

FYI, fixing this didn't help resolution times in my specific test cases, but I assume it might help in some other use cases.

[uranusjr]

Probably should still fix this. Also I think the type of requested_order can just be float?

[notatallshaw]

Probably should still fix this.

Would you like me to submit a PR? Might take up to a few weeks as I only do this in my spare time.

Also I think the type of requested_order can just be float?

requested_order is derived from the values of self._user_requested which has a type of Dict[str, int]. I know that mypy considers int a subtype of float but the 2 objects do have a different set of methods so the current approach is probably good for other type checkers.

[uranusjr]

A PR would be most awesome. We’re in no hurry now since this will go into the next release cycle the soonist in October anyway.

requested_order is derived from the values of self._user_requested which has a type of Dict[str, int].

We only care about __add__ and __gt__ (or is it __lt__? I don’t remember what min uses), so the easiest solution is to simply always use float (e.g. requested_order = 1.0 if identifier in self._user_requested else math.inf).

[notatallshaw]

A PR would be most awesome. We’re in no hurry now since this will go into the next release cycle the soonist in October anyway.

requested_order is derived from the values of self._user_requested which has a type of Dict[str, int].

We only care about __add__ and __gt__ (or is it __lt__? I don’t remember what min uses), so the easiest solution is to simply always use float (e.g. requested_order = 1.0 if identifier in self._user_requested else math.inf).

I don't think this is right, to maintain the feature of user order requested_order should be more than the possible values of 1.0 or math.inf (in fact if it was only those 2 values I would suggest it be a bool).

I think it would need to be requested_order = float(self._user_requested.get(identifier, math.inf)). I could be missing something though.

Random side-note: I learnt today that Python can sort an integer larger than a maximum floating value and math.inf. E.g you can run max([2**2**2**2**2, math.inf]). I think this is interesting because as far as I'm aware math.inf has no integer representation and 2**2**2**2**2 has no float representation.

[uranusjr]

I think it would need to be requested_order = float(self._user_requested.get(identifier, math.inf)). I could be missing something though.

You’re right, I messed that up. And yes, you can just use a very large int as well. The usual approach is sys.maxint, which is the largest int that a platform can handle without the bigint implementation to kick in (which is significantly slower than native int).

[notatallshaw]

I've tried setting up local pip development environment and create a pull request but it seems like Windows tests don't fully work: https://github.com/notatallshaw/pip/runs/3515183542?check_suite_focus=true ?

Is this a known issue I can ignore and direct the pull request towards https://github.com/pypa/pip ?

Also is there somewhere I should be adding a unit test for this?

[uranusjr]

The signal only works in main thread error is a known bug in pytest-xdist and not related to pip itself. It happens from time to time (and now only on Windows), you can safely it.

[notatallshaw]

Made first attempt at pull request.

Also it seems like running test suite locally on Windows is just a no go for Pip? The best I've been able to get so far is:

690 failed, 1448 passed, 73 skipped, 15 xfailed, 1 xpassed, 3496 warnings, 32 errors in 578.10s 

About 7 or 8 of the failures are unit tests, and when I look at why they failed they don't seem to make any sense, I've figured out one requires your machine to be in UTC but no idea why. The integration tests give hundreds of failures with no clear pattern. I'll try setting up a WSL 2 environment I guess.

[pfmoore]

I have no problem running the tests on my Windows 10, UK English PC. Everything passes (or xfails) in my environment. I don't have time right now to do a lot of digging, I'm afraid, but if you've got specific issues, I can try to take a look over the next week. The one that seems to need UTC sounds interesting - I'm in the UK but I have run the tests while I have summer time set, so I think it may be a little more than just assuming UTC...

Or if you'd rather switch to Linux (WSL) for your tests, that's fine. Whatever works best for you.

[pradyunsg]

Also it seems like running test suite locally on Windows is just a no go for Pip? The best I've been able to get so far is:

Could you file a new issue, with the complete output of the test run?

[notatallshaw]

Thanks everyone for your inputs, here is what I will do:

1. Do another fresh install on Windows following these instructions: https://pip.pypa.io/en/latest/development/getting-started/
2. Do a fresh install in WSL 2 and compare the results

If 1) I am still getting a large number of errors on Windows, and 2) I am not getting a large number of errors in WSL 2, I will make a new issue documenting all the steps I followed and the full output.

If I am getting a large number of errors in both environments it's probably a failure in my ability to follow instructions and I will spend more time trying to figure out what I've done wrong.

[uranusjr]

We should split the Windows test setup topic into a separate issue. I am pretty sure the tests work on Windows (I’m developing pip on native Windows as well).

[edmorley]

Can this be closed now that #10482 has merged? :-)

[uranusjr]

Yes, thanks for noticing it!