vision icon indicating copy to clipboard operation
vision copied to clipboard

Help make test dependent on randint compatible with python 3.12

Open hmaarrfk opened this issue 2 years ago • 3 comments

It seems that python 3.12 is a little stricter on the inputs of randint.

I hope this helps

In [1]: import random

In [2]: random.randint(10, 10/2)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Cell In[2], line 1
----> 1 random.randint(10, 10/2)

File ~/miniforge3/envs/py12/lib/python3.12/random.py:336, in Random.randint(self, a, b)
    332 def randint(self, a, b):
    333     """Return random integer in range [a, b], including both end points.
    334     """
--> 336     return self.randrange(a, b+1)

File ~/miniforge3/envs/py12/lib/python3.12/random.py:312, in Random.randrange(self, start, stop, step)
    309     raise ValueError("empty range for randrange()")
    311 # Stop argument supplied.
--> 312 istop = _index(stop)
    313 width = istop - istart
    314 istep = _index(step)

TypeError: 'float' object cannot be interpreted as an integer

hmaarrfk avatar Nov 16 '23 04:11 hmaarrfk

:link: Helpful Links

:test_tube: See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/vision/8119

Note: Links to docs will display an error until the docs builds have been completed.

:x: 14 New Failures, 18 Unrelated Failures

As of commit a4bc6b3022d57f33683aae364536e26d6c1e4cd2 with merge base dd66a9d83d499788a7589cc4bf0d5fb6bc3a2127 (image):

NEW FAILURES - The following jobs have failed:

FLAKY - The following jobs failed but were likely due to flakiness present on trunk:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

pytorch-bot[bot] avatar Nov 16 '23 04:11 pytorch-bot[bot]

Thanks for the PR @hmaarrfk .

We don't support 3.12 yet but when the time comes, we'll be happy to re-consider this PR.

I genuinely hope that this is not here to stay TBH

>>> random.randint(0, 4/2)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/nicolashug/.miniconda3/envs/fuck/lib/python3.12/random.py", line 336, in randint
    return self.randrange(a, b+1)
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/home/nicolashug/.miniconda3/envs/fuck/lib/python3.12/random.py", line 312, in randrange
    istop = _index(stop)
            ^^^^^^^^^^^^
TypeError: 'float' object cannot be interpreted as an integer

NicolasHug avatar Nov 16 '23 09:11 NicolasHug

thanks for looking at this so quickly and taking the time to test on your machine.

this seems intentional. see notes about automatic conversation.

https://docs.python.org/3/library/random.html#functions-for-integers

integer division resolves this issue easily. the PR is 8 characters that go in the right direction toward 3.12 support without hurting other versions in any way. it is contained in the test suite, and it is obvious from inspection that the type of the inputs to the division operator are integers.

I mostly just wanted to help given that I had tried to see how far along python 3.12 one could go. I'm not asking you to accelerate your timeline in any way.

take it or leave it I guess.

hmaarrfk avatar Nov 16 '23 11:11 hmaarrfk

Just a kind ping hoping you can consider the introduction of a total of 10 characters.

hmaarrfk avatar Feb 25 '24 14:02 hmaarrfk

Thank you for the ping @hmaarrfk . We'll merge the PR in due time, when 3.12 support becomes official for Pytorch and torchvision.

I'm sure you're experienced enough to know that the complexity, debt and criteria for inclusion of a PR doesn't correlate with its number of characters.

NicolasHug avatar Feb 26 '24 11:02 NicolasHug

Failures are unrelated, let's merge

NicolasHug avatar Mar 13 '24 11:03 NicolasHug