SG-32657 Enable support for Python 3.10 in CI#920
SG-32657 Enable support for Python 3.10 in CI#920carlos-villavicencio-adsk merged 19 commits intomasterfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## master #920 +/- ##
==========================================
+ Coverage 78.91% 78.95% +0.03%
==========================================
Files 182 183 +1
Lines 20083 20163 +80
==========================================
+ Hits 15849 15920 +71
- Misses 4234 4243 +9
... and 10 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
09a9598 to
4240b18
Compare
| for version in versions: | ||
| if LooseVersion(version) > LooseVersion(latest): | ||
| latest = version | ||
| with suppress_known_deprecation(): |
There was a problem hiding this comment.
Wow, I love this function!
Can you add a comment to say for which Python version we had this?
There was a problem hiding this comment.
Agree with that , that context manager is great
| is_version_newer, | ||
| is_version_older_or_equal, | ||
| is_version_newer_or_equal, | ||
| suppress_known_deprecation, |
There was a problem hiding this comment.
Again, not a blocker but I really wonder why bother listing all these modules, classes, and methods in this file instead of using Python's default import system? (from utils import version then version.suppress_known_deprecation)
There was a problem hiding this comment.
From what I've seen it's just to simplify the importing workflow. Maybe that was the original code style for tk-core.
| warnings.filterwarnings( | ||
| action="default", | ||
| category=DeprecationWarning, | ||
| message="distutils Version classes are deprecated.", |
There was a problem hiding this comment.
I am confused now. Should we not use this method then?
There was a problem hiding this comment.
You can find more information in the PR description. To sum up, distutils is now deprecated, and starting from Python 3.10 we will use the distutils copy that is shipped on setuptools.
There was a problem hiding this comment.
Ok but this is a temporary solution, right? At some point, we need to migrate to packaging.version.parse, no?
If so, do we have a plan for that (a Jira ticket)? And why not now (too big?)?
There was a problem hiding this comment.
Yes, it's too big. At first, I tried the migration to packaging.version.parse but the existing code for tk-core, tk-nuke, and probably more components rely on the existing logic (e.g. comparisons for DCC versions) and they break because it doesn't always comply with PEP 440.
We won't be able to easily move to packaging. I'd suggest to keep using LooseVersion from the legacy distutils and keep suppressing the deprecation. Or fork the class code ourselves and strip out the deprecation.
packaging.version.parsebecause many TK components depend ofdistutils.version.LooseVersion. To handle the distutils deprecation, we're switching to the bundled version living on setuptools >= 60. The deprecation warning is still there, so we're suppressing just this one.isSet()