-
Notifications
You must be signed in to change notification settings - Fork 622
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
Fix Imf/Iex/IlmThread namespaces in python bindings and website code #1568
Fix Imf/Iex/IlmThread namespaces in python bindings and website code #1568
Conversation
Use OPENEXR_NAMESPACE, IEX_NAMESPACE, ILMTHREAD_NAMESPACE instead of Imf, Iex, IlmThread, to support custom settings. Signed-off-by: Cary Phillips <cary@ilm.com>
This may be fine for now, but I feel like there is a more sensible way to structure the namespaces and namespace override options. I don't need to use these macros throughout the code in my other projects. There should only be two mentions of the "INTERNAL" namespaces -- once in the CMake to allow users a way to override it for a custom namespace, and then a second in a header that aliases the publicly visible namespace to that one. All user code, and all other internal code, ought to be able to safely say Iex::foo and it's fine, even though there is a custom namespace behind it all. |
This broke when I added the website example code to the build, and looking at the code more carefully now, it makes inconsistent use of explicit namespaces. What's the prevailing sense of whether example code in documentation should use explicit namespaces? Or assume there's an implicit
This code on the website is an implicit statement of preferred usage policy. Should |
Requires this in all.cpp: namespace Iex = IEX_NAMESPACE; Signed-off-by: Cary Phillips <cary@ilm.com>
Signed-off-by: Cary Phillips <cary@ilm.com>
Signed-off-by: Cary Phillips <cary@ilm.com>
I've removed the explicit Iex:: namespace from the example website code, and added This compiles with both default and custom namespaces, so resolves #1567. Let's flag as a separate issue both cleaning up the namespace declarations and editing the example code to both show best practices as well as work under all build configurations. |
7c5c312
into
AcademySoftwareFoundation:main
…cademySoftwareFoundation#1568) * Fix Imf/Iex/IlmThread namespaces in python bindings and website code Use OPENEXR_NAMESPACE, IEX_NAMESPACE, ILMTHREAD_NAMESPACE instead of Imf, Iex, IlmThread, to support custom settings. Signed-off-by: Cary Phillips <cary@ilm.com> * Use Iex:: namespace in example code instead of IEX_NAMESPACE Requires this in all.cpp: namespace Iex = IEX_NAMESPACE; Signed-off-by: Cary Phillips <cary@ilm.com> * use #define Iex Signed-off-by: Cary Phillips <cary@ilm.com> * remove explicit Iex namespace Signed-off-by: Cary Phillips <cary@ilm.com> --------- Signed-off-by: Cary Phillips <cary@ilm.com>
* Propagate OPENEXR_INSTALL_PKG_CONFIG to internal Imath (#1531) * Propagate OPENEXR_INSTALL_PKG_CONFIG to internal Imath If OpenEXR is installing a pkg-config file, then the internal Imath build (if there is one) should install it, too. Also, add an explicit release version variable to the .pc file. This is in preparation for the python wheel build to pick up the version settings from the pkg-config files. Signed-off-by: Cary Phillips <cary@ilm.com> * remove version= from OpenEXR.pc Signed-off-by: Cary Phillips <cary@ilm.com> --------- Signed-off-by: Cary Phillips <cary@ilm.com> * Remove check for _MSC_VER in internal_cpuid.h (#1528) As noted in #1445, _MSC_VER should only be used to detect msvc, not for Windows. The _WIN32 check should be sufficient. The extra check for _MSC_VER fails when cross-compiling from Linux to Windows. Signed-off-by: Cary Phillips <cary@ilm.com> * Fix warnings in multipartExamples.cpp (#1533) * Fix warnings in multipartExamples.cpp Signed-off-by: Cary Phillips <cary@ilm.com> * fix indentation Signed-off-by: Cary Phillips <cary@ilm.com> --------- Signed-off-by: Cary Phillips <cary@ilm.com> * Fix warnings from cross-compiling with x86_64-w64-mingw32-gcc-posix (#1534) * Fix compiler warnings from x86_64-w64-mingw32-g++-posix * MSVCRT doesn't recognize printf("%l") so PRIu64 doesn't work, it expects "%I64u". * Add to the cases that disable a strncpy warning Signed-off-by: Cary Phillips <cary@ilm.com> * %zu for size_t Signed-off-by: Cary Phillips <cary@ilm.com> * revert attempt to fix PRIu64 warnings Signed-off-by: Cary Phillips <cary@ilm.com> * revert blank line Signed-off-by: Cary Phillips <cary@ilm.com> --------- Signed-off-by: Cary Phillips <cary@ilm.com> * Fix OPENEXR_VERSION_HEX (#1539) * Fix OPENEXR_VERSION_HEX * fix syntax error * remove unnecessary uint32_t cast * add validation test Signed-off-by: Cary Phillips <cary@ilm.com> * Add #if statement with OPENEXR_VERSION_HEX Signed-off-by: Cary Phillips <cary@ilm.com> --------- Signed-off-by: Cary Phillips <cary@ilm.com> * OPENEXR_INSTALL_PKG_CONFIG is on by default, even on Windows (#1541) There seems to be no downside to generating the OpenEXR.pc file even on Windows, so for consistency, apply the setting to all platforms. Signed-off-by: Cary Phillips <cary@ilm.com> * Remove SPDX license identifier in LICENSE.md Signed-off-by: Cary Phillips <cary@ilm.com> * Python wheel setup gets version from OpenEXR.pc/Imath.pc (#1536) * Python wheel setup gets version from OpenEXR.pc/Imath.pc OpenEXR and Imath have indepdenent versions that may not match. The .pc files appear to be the simplest way to extract the versions and library suffixes. Running pkg-config itself doesn't work on Windows, hence the manual file parsing. Signed-off-by: Cary Phillips <cary@ilm.com> * = Signed-off-by: Cary Phillips <cary@ilm.com> * Remove -DOPENEXR_INSTALL_PKG_CONFIG=ON since it's now on by default Signed-off-by: Cary Phillips <cary@ilm.com> --------- Signed-off-by: Cary Phillips <cary@ilm.com> * Default value for chromaticities attribute constructor in exrstdattr (#1540) Signed-off-by: Cary Phillips <cary@ilm.com> * Update openexr_deps.bzl (#1565) Signed-off-by: Vertexwahn <julian.amann@tum.de> * Bazel: Improve module (#1562) Signed-off-by: Vertexwahn <julian.amann@tum.de> * Set build-shared:OFF for Static build (#1557) Looks like this somehow we've been building shared when we meant to be building static. Signed-off-by: Cary Phillips <cary@ilm.com> * Omit OPENEXR_IMAGES_TAG from test image url if empty (#1560) This allows the url for the test images to reference a local file: cmake -DOPENEXR_IMAGES_REPO=file:///my/clone/of/openexr-images -DOPENEXR_IMAGES_TAG="" which avoids the remote downloading of the test images at cmake time. Also, mention the test images in the install docs. Signed-off-by: Cary Phillips <cary@ilm.com> * Clean up handling of libdeflate when linking static (#1561) * Set EXR_DEFLATE_LIB properly for static linking This leads OpenEXRCore/CMakeLists.txt to do: target_link_libraries(OpenEXRCore PUBLIC libdeflate) instead of: target_link_libraries(OpenEXRCore PUBLIC PkgConfig::deflate) which is not recognized. With a target link library of `PkgConfig::deflate`, the static build of the OpenEXR libraries builds successfully, but configuring an application against those static libraries fails, saying it can't find `PkgConfig::deflate`. Signed-off-by: Cary Phillips <cary@ilm.com> * Add -ldeflate to pkgconfig for static builds When static linking and referencing an external libdeflate (i.e. not fetching and building internally), OpenEXR.pc needs -ldeflate. This also adds a test for this condition to the CI's validation script. Signed-off-by: Cary Phillips <cary@ilm.com> --------- Signed-off-by: Cary Phillips <cary@ilm.com> * Don't trigger ci/bazel/ossfuzz builds on pushes/PRs to src/wrappers (#1532) No need to rebuild/test OpenEXR on changes to the python wheels. Signed-off-by: Cary Phillips <cary@ilm.com> * Set minimal permissions for workflow python-wheels.yml (#1530) The exact same changes done on other workflows through the PR #1417 Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com> * Release notes for 3.2.1 Signed-off-by: Cary Phillips <cary@ilm.com> * Fix Imf/Iex/IlmThread namespaces in python bindings and website code (#1568) * Fix Imf/Iex/IlmThread namespaces in python bindings and website code Use OPENEXR_NAMESPACE, IEX_NAMESPACE, ILMTHREAD_NAMESPACE instead of Imf, Iex, IlmThread, to support custom settings. Signed-off-by: Cary Phillips <cary@ilm.com> * Use Iex:: namespace in example code instead of IEX_NAMESPACE Requires this in all.cpp: namespace Iex = IEX_NAMESPACE; Signed-off-by: Cary Phillips <cary@ilm.com> * use #define Iex Signed-off-by: Cary Phillips <cary@ilm.com> * remove explicit Iex namespace Signed-off-by: Cary Phillips <cary@ilm.com> --------- Signed-off-by: Cary Phillips <cary@ilm.com> * Bump version to 3.2.1 Signed-off-by: Cary Phillips <cary@ilm.com> --------- Signed-off-by: Cary Phillips <cary@ilm.com> Signed-off-by: Vertexwahn <julian.amann@tum.de> Signed-off-by: Diogo Teles Sant'Anna <diogoteles@google.com> Co-authored-by: Vertexwahn <julian.amann@tum.de> Co-authored-by: Diogo Teles Sant'Anna <diogoteles@google.com>
Use OPENEXR_NAMESPACE, IEX_NAMESPACE, ILMTHREAD_NAMESPACE instead of Imf, Iex, IlmThread, to support custom settings.
Address #1567.