tinygrad icon indicating copy to clipboard operation
tinygrad copied to clipboard

Move from python3.8 to python3.10 and from Union to |

Open smurfd opened this issue 1 year ago • 4 comments

Hey,

Saw that maby a move from Python 3.8 to Python 3.10 was interesting. Enforced python3.10 version for some of the workflows that did not have a version. Left the ones with python3.11 or python3.12 untouched.

Switched from typing Union[type1, type2] to type1 | type2

ran tests locally, Did a stupid test for the match, case also available in python3.10 https://bpa.st/KQKA

smurfd avatar May 21 '24 17:05 smurfd

3rd time is the charm?

smurfd avatar May 21 '24 17:05 smurfd

That could be done with the ruff pyupgrade rule https://docs.astral.sh/ruff/rules/non-pep604-annotation/, and it would also replace Optional[X] with X | None.

You should probably also update https://github.com/tinygrad/tinygrad/blob/master/setup.py#L26 with python>=3.10

PhillCli avatar May 21 '24 19:05 PhillCli

This branch currently is behind tinygrad/master. The line count difference bot is disabled.

github-actions[bot] avatar May 22 '24 17:05 github-actions[bot]

Thanks PhillCli, ruff is great :) added those changes aswell.

smurfd avatar May 22 '24 17:05 smurfd

Closing this, tinygrad will follow pytorch and EOL of Python3.8 which is 2024-10 https://peps.python.org/pep-0569/

https://github.com/pytorch/pytorch/issues/120718

when its time, you should be able to do: add to ruff.toml "UP007" # non-pep604-annotation

update setup.py python_requires='>=3.10',

Then you should be able to run:

python3 -m ruff --target-version py310 --unsafe-fixes --diff pyupgrade .

smurfd avatar May 24 '24 17:05 smurfd

IIRC, it could work in python3.7+ codebase, at the cost of adding at the top of modules (that use that new syntax), the following snippet:

from __future__ import annotations

PhillCli avatar May 24 '24 17:05 PhillCli

edit: i need to change like: sint = int | Variable | MulNode | SumNode to sint: int | Variable | MulNode | SumNode

lets see if i get it working, then i reopen it...

Thanks again PhillCli, yes for simple cases that works. But for some reason here it does not. Tried to add

from __future__ import annotations

to everywhere i had touched on top of the file, but tests failed alot. And evry file in the callstack also has it on top, but As an example:

$ python3.8 -m pytest test/test_ops.py 
==================================================== test session starts =====================================================
platform darwin -- Python 3.8.19, pytest-8.2.0, pluggy-1.5.0
rootdir: /Users/willygorillatg/tinygrad
plugins: hypothesis-6.100.1, xdist-3.5.0
collected 0 items / 1 error                                                                                                  

=========================================================== ERRORS ===========================================================
_____________________________________________ ERROR collecting test/test_ops.py ______________________________________________
test/test_ops.py:5: in <module>
    from tinygrad.helpers import getenv, IMAGE, DEBUG, CI
tinygrad/__init__.py:2: in <module>
    from tinygrad.tensor import Tensor                            # noqa: F401
tinygrad/tensor.py:12: in <module>
    from tinygrad.lazy import LazyBuffer
tinygrad/lazy.py:6: in <module>
    from tinygrad.ops import LoadOps, UnaryOps, BinaryOps, TernaryOps, ReduceOps, Op, exec_alu, python_alu
tinygrad/ops.py:8: in <module>
    from tinygrad.shape.symbolic import Variable, sint
tinygrad/shape/symbolic.py:310: in <module>
    sint = int | Variable | MulNode | SumNode
E   TypeError: unsupported operand type(s) for |: 'type' and 'type'
================================================== short test summary info ===================================================
ERROR test/test_ops.py - TypeError: unsupported operand type(s) for |: 'type' and 'type'

no worries, ill come back in october ;)

smurfd avatar May 24 '24 19:05 smurfd

I think a discussion needs to take place if this project should follow SPEC 0 or not.

EwoutH avatar May 28 '24 19:05 EwoutH