Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python3 Compatibility #134

Merged
merged 8 commits into from
Mar 14, 2021
Merged

Python3 Compatibility #134

merged 8 commits into from
Mar 14, 2021

Conversation

PumpingPixels
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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

@PumpingPixels
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor Author

PumpingPixels commented Jan 24, 2021

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 jonahfriedman merged commit 3b7d430 into Psyop:master Mar 14, 2021
@jonahfriedman
Copy link
Contributor

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.

@PumpingPixels PumpingPixels deleted the python3 branch March 15, 2021 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants