search

Psyop / Cryptomatte

Cryptomatte Nuke plugin, Fusion plugin, sample images, and specification
BSD 3-Clause "New" or "Revised" License
632 stars 151 forks source link

Python3 Compatibility #134

Closed PumpingPixels closed 3 years ago

PumpingPixels commented 3 years ago

Hey there, I was doing some Python 3 transition work for my own code and since I was in the flow already I decided to port the Nuke implementation for Cryptomatte as well.

The changes are compatible with both Python 2 and Python 3. Integration tests run fine on Nuke 11 and 12 as well as the latest Nuke 13 Python3 Alphas (CentOS 7). I also encountered no problems in a bunch of manual testing in the meantime.

jonahfriedman commented 3 years ago

Hi, thanks for this contribution!

I found a couple issues with it, which I fixed in 1f5f3ba and c2cdf05. With those, I think everything works in both Python 2 and Python 3 nukes simultaneously. I have them in a python3 branch in this repo. I think this will mean changing what versions of Nuke we support as well, but I'm not sure.

Still needed, but I won't get to work on it for a bit - need to apply these changes to the wildcards branch for the beta version.

jfpanisset commented 3 years ago

When this goes in I'm happy to add an entry to the VES Python 3 support tracker.

PumpingPixels commented 3 years ago

Thanks for looking into it. Regarding supported Nuke version, as far as I am aware Nuke 13 will be Python 3 only, so without the changes, the current master branch is only compatible to Nuke 7-12 as soon as that is released, the python3 changes do support Nuke 7+ as before.

Is it correct that the latest updates to the wildcard feature is in the dev branch? If I find the time I would look into merging the python3 changes into it.

PumpingPixels commented 3 years ago

I made a small additional change to improve the support for older Nuke versions based on Python 2.6. I noticed the integration tests throw some errors on these, which is already the case for the latest master branch.

jonahfriedman commented 3 years ago

Thanks JF and @PumpingPixels.

I have updated both the Python3 and dev branches. The main branch will be released as 1.3.0 (e178e35), and the wildcards beta is released as 1.4.0-beta4. I think we're about ready to release the main one too, perhaps after doing a bit of manual testing to sanity check the automated tests.

As for Nuke 6, I can't test with it and I realized we already only claim support for Nuke 7+, so I think we can just say Nuke 6 is not supported.

PumpingPixels commented 3 years ago

I am aware that only support for 7+ is claimed, but Nuke versions based on Python 2.6 include Nuke 7. The test throw a bunch of errors with it, tested on Nuke 7.0v6 on Windows 10 (v2004). Having a quick look this seems to be due to the fact the the tests use assertion methods introduced in Python 2.7.

This is going a bit off topic, my point was that with the latest additions, the Python3 compatibility doesn't introduce any errors that aren't already present in the master branch, at least in regards to the tests.

jonahfriedman commented 3 years ago

This is now released, thank you for the contribution!

I've increased the minimum version to Python 2.7 / put Nuke 8 in the release notes. Realistically, I can't test Nuke 8 anymore either.