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

ImageInput and ImageOutput docs updated to Python 3. #3866

Merged
merged 1 commit into from
Jun 7, 2023

Conversation

ziggycross
Copy link
Contributor

@ziggycross ziggycross commented Jun 7, 2023

Description

Fixes #3846 by updating Python examples.

Current code examples for ImageInput and ImageOutput raise a SyntaxError in Python 3:

File "<cwd>\main.py", line 13
    return
    ^^^^^^
SyntaxError: 'return' outside function

It seems these code examples might be for Python 2.7. Perhaps it would be worth creating an extra tab instead of replacing the old code?

Tests

Tested on Python 3.11, no issues.

Checklist:

  • I have read the contribution guidelines.
  • If this is more extensive than a small change to existing code, I
    have previously submitted a Contributor License Agreement
    (individual, and if there is any way my
    employers might think my programming belongs to them, then also
    corporate).
  • I have updated the documentation, if applicable.
  • I have ensured that the change is tested somewhere in the testsuite
    (adding new test cases if necessary).
  • My code follows the prevailing code style of this project.

Copy link
Collaborator

@lgritz lgritz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix!

@lgritz
Copy link
Collaborator

lgritz commented Jun 7, 2023

I think that it's not so much a "python 2 vs 3" thing, as it is:

(a) The examples in the docs are program fragments, not literal full working programs. So 'return' would be fine if this code were inside a function, but doesn't make sense in the main script body.

(b) In many cases, I started out with C++ (also fragments, not compilable complete C++ programs) and hand-translated them to make the python equivalents. But I didn't always test them, so probably made plenty of little mistakes, compounded by the fact that I don't use Python every day like I do C++, so my Python can be a little un-fluent, especially when I'm not testing it.

In the longer run, I think it would be interesting to set up some kind of fully automated scheme in which most of the sample code in the docs gets run as testsuite items. Or alternately, the other way around -- that they live entirely in the testsuite and get "included" into the docs when code examples are needed.

@lgritz lgritz added this pull request to the merge queue Jun 7, 2023
@lgritz lgritz removed this pull request from the merge queue due to a manual request Jun 7, 2023
Merged via the queue into AcademySoftwareFoundation:master with commit 116a1f1 Jun 7, 2023
@ziggycross
Copy link
Contributor Author

You're totally right - I don't have much experience with Python 2 and suspected it might be a compatibility issue. But after looking into it, it doesn't seem the return keyword was much different in v2, so that makes sense if it was just un-tested code.

I don't know much about testsuite but would be happy to see what needs to be done to link it up with the docs.

@ziggycross ziggycross deleted the docs-python-3 branch June 7, 2023 23:14
@lgritz
Copy link
Collaborator

lgritz commented Jun 7, 2023

I have no doubt that there are also lots of examples in the docs that I wrote in the python2 days and are not valid python3.

I bet if I try out what I'm talking about with the testsuite/docs connection on ONE example, then with that existing as a template, it should be fairly easy to convert a lot of the rest of the code examples in the docs to be like that.

@lgritz
Copy link
Collaborator

lgritz commented Jun 7, 2023

I meant, if I do one, it should be easy for OTHER PEOPLE to do the same thing to all the other examples.

lgritz pushed a commit to lgritz/OpenImageIO that referenced this pull request Jun 7, 2023
…Foundation#3866)

## Description

Fixes AcademySoftwareFoundation#3846 by updating Python examples.

Current code examples for
[ImageInput](https://openimageio.readthedocs.io/en/latest/imageinput.html)
and
[ImageOutput](https://openimageio.readthedocs.io/en/latest/imageoutput.html)
raise a SyntaxError in Python 3:
```python
File "<cwd>\main.py", line 13
    return
    ^^^^^^
SyntaxError: 'return' outside function
```

It seems these code examples might be for Python 2.7. Perhaps it would
be worth creating an extra tab instead of replacing the old code?

## Tests

Tested on Python 3.11, no issues.

## Checklist:

- [x] I have read the [contribution
guidelines](https://github.com/OpenImageIO/oiio/blob/master/CONTRIBUTING.md).
- [ ] If this is more extensive than a small change to existing code, I
  have previously submitted a Contributor License Agreement

([individual](https://github.com/OpenImageIO/oiio/blob/master/src/doc/CLA-INDIVIDUAL),
and if there is any way my
  employers might think my programming belongs to them, then also

[corporate](https://github.com/OpenImageIO/oiio/blob/master/src/doc/CLA-CORPORATE)).
- [x] I have updated the documentation, if applicable.
- [ ] I have ensured that the change is tested somewhere in the
testsuite
  (adding new test cases if necessary).
- [x] My code follows the prevailing code style of this project.
@ziggycross
Copy link
Contributor Author

I have no doubt that there are also lots of examples in the docs that I wrote in the python2 days and are not valid python3.

I bet if I try out what I'm talking about with the testsuite/docs connection on ONE example, then with that existing as a template, it should be fairly easy to convert a lot of the rest of the code examples in the docs to be like that.

If you do this, I'd be happy to copy the rest over, at least for the Python examples.

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.

[DOC] ImageOutput: Writing Images wrong Python example code
2 participants