Conversation
- Fix horribly wrong usage instructions - Format everything for PEP8 - Fix wrong comments leftover from C/C++ port - Ensure images are installed so the tutorials work there too as-is - Updated imageio usage to avoid deprecation warnings - Add README_python.md to the install
steven-johnson
requested review from
abadams,
jrk,
rootjalex,
shoaibkamil,
vksnk and
zvookin
|
(Removed now-irrelevant comment about imageio, obsoleted/fixed by #7125) |
| assert output_buffer[xx, yy] == 192 | ||
|
|
||
|
|
||
| # If you like, you can put multiple Generators in the same source file. This |
There was a problem hiding this comment.
Are the first two sentences in this paragraph still helpful in the python context?
There was a problem hiding this comment.
Nope. Removing.
steven-johnson
added
the
skip_buildbots
(Cherry-picked from WIP #7109) It seems that as of imageio v2.16, they offer new and old apis, which must be imported as imageio.v3 or imageio.v2 respectively; if you just import and use the naked imageio but have 2.16+ installed, you get a noisy and puzzling deprecation warning emitted to stdout, which is not great, especially for tutorials. All the options here seem bad: - We could just leave stuff as-is, but that would mean that folks running with a newer (Feb 2022 +) install of imageio would get a distracting deprecation warning - We could play nicely with both older and newer versions of imageio by wrapping the import in a try block; that would work, but would be a distracting wart at the top of each tutorial - We could settle on using either the v2 or v3 API, but that means that folks with older imageio installs fail outright - For the moment, I settled on the last one (using v2), along with requirements.txt... which forced me to upgrade the version on all the buildbots, since they were mostly running 2.9. I'd welcome feedback on this.
|
What's the state of this? Can I help to bring this PR over the finish line? I have the feeling that already merging this as-is is a huge improvement over the status quo and would have made my life starting with halides python bindings a lot easier :) |
|
Steven is currently away, so it's unlikely to get any attention from him in the next month or so. If you'd like to take it over and get it over the finish line, that would be welcome. |
|
What would be needed for this? Just a rebase to the current main? |
|
For a start yeah, but the thing it really needs is for a python user of Halide to go through it and check it all makes sense. |
3 participants
(This PR is not yet complete; opening as a Draft PR to get discussion and input)
The tutorials for the python bindings are in need of a lot of love. As of right now, this PR does this:
for PEP8 (mostly, though some >80c lines are there)There are a couple of gotchas here, though:
(1) A lot of the tutorials were never ported to Python; we only go thru 14, while C++ goes up to 21. This isn't likely a big deal, porting them is mostly plug-n-chug. But...
(2) The structure of the stuff in the tutorials relating to AOT and Generators needs a complete overhaul. When the tutorials were first written, Generators didn't exist, so lesson 10 talks about AOT via direct calls to
compile_to_file(); lesson 11 is about cross-compilation, using the same approach; lesson 12 is about GPU usage, and as a convenience there, it uses a generator-like class that is not a Generator.At this point in time we really want to steer people towards Generators for AOT use, so this PR actually removes lessons 10-12 from Python entirely for now. What I think we (well, I) should do here is this:
I think this would be a more useful ordering of lessons for recent Halide, but it would (at least initially) diverge from the C++ tutorials in terms of the numbering, etc. (In the medium-to-long term we would bring the C++ versions up to date with the Python versions.)
What do folks think?