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

Enhancing ocio config embedding of looks #25

Merged
merged 19 commits into from
Sep 14, 2024

Conversation

jakubjezek001
Copy link
Member

@jakubjezek001 jakubjezek001 commented Jul 30, 2024

  • selenium downgrade
  • enhancing AYON ocio look operator
  • enhancing AYON ocio look operator testing
  • testing config file adding logging and stagig file fixture

TODO:

  • enhancing AYON hiero effect operator and testing:
    • adding target path like in AYON ocio look operator (for AYON publishing workflow)
    • testing wider coverage

…ion setting in start.ps1

- Updated selenium dependency to version 4.16.0
- Added pytest as a dev-dependency in poetry
- Removed specific Poetry version setting from start.ps1
WIP on enhancement/renderer-basic

- Added a list of search paths
- Initialized variables and operators
- Implemented methods to handle search paths
- Updated file loading process with target path option
Adjusted test method to use kwargs and results dictionaries for better parameter handling, updated assertions accordingly.
…sor.

Increased clarity and functionality in OCIO configuration handling.
- Updated method to clear operators
- Modified typing import in a file
- Added logging functionality using Python's logging module
- Changed the setting to keep test results after session ends
Changed the configuration to set KEEP_TEST_RESULTS to False in order to prevent keeping test results after each run.
…or object.

- Refactored debug logs and variable names for clarity
- Updated assertions to use the renamed 'processor' object
- Added new test parameter structure with file paths and expected results.
- Adjusted test method to accept kwargs and results instead of just a path.
- Updated processor initialization and debug log messages.
- Modified assertions to compare processor attributes with expected results.
- Update target directory path handling and logging
-  Adjust file paths accordingly.
- Add debug logs for new file paths and processor attributes.
- Refactor test cases to match changes made in the main code.
Copy link
Collaborator

@tweak-wtf tweak-wtf left a comment

Choose a reason for hiding this comment

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

just some notes here and there.
tbh... as long as the ci testing doesn't fail we're pretty safe to know we're not breaking other parts when merging. And they do run without any issues!

so you have my blessing :D

lablib/generators/ocio_config.py Outdated Show resolved Hide resolved
lablib/generators/ocio_config.py Outdated Show resolved Hide resolved
lablib/processors/ayon_hiero_effect_file.py Show resolved Hide resolved
pyproject.toml Show resolved Hide resolved
pyproject.toml Show resolved Hide resolved
- Added support for specifying a target view space in the OCIO Config file.
- Refactored code to handle default target view space assignment.
- Improved code formatting and removed redundant comments.
- Updated method to append search paths instead of clearing and appending.
- Improved logic for handling multiple search path arguments.
@jakubjezek001 jakubjezek001 force-pushed the slate-testing-and-selenium-downgrading branch from bd6892f to 0d91f37 Compare August 8, 2024 12:51
…roughout the code for consistency.

- Removed unnecessary _ocio_transforms list
- Replaced instances of _ocio_transforms with _operators throughout the code for consistency
- Added import statement for PyOpenColorIO.
- Updated test_OCIOConfigFileGenerator method to include additional tests related to environment variables, config file path, color command, config file content, environment variables in OCIO config, search paths with environment variables, context-related colorspace/look/view.
OCIO api is having bug which is not allowing adding search paths and environment variables via official methods. This way we are adding it via serialisation but it will need to be replaced in future with proper way.
@jakubjezek001 jakubjezek001 marked this pull request as ready for review September 10, 2024 12:33
@jakubjezek001 jakubjezek001 removed the request for review from iLLiCiTiT September 10, 2024 12:34
f"--ociolook:from=\"{self.working_space}\""
f":to=\"{self.working_space}\""
),
(f'--ociolook:from="{self.working_space}"' f':to="{self.working_space}"'),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
(f'--ociolook:from="{self.working_space}"' f':to="{self.working_space}"'),
f'--ociolook:from="{self.working_space}":to="{self.working_space}"',

@tweak-wtf
Copy link
Collaborator

chiming in real quick.
the only reason this is not already merged yet is that fd64571 introduced a bug.
the stacktrace is available here: https://github.com/ynput/LabLib/actions/runs/10387476368/job/28761036338?pr=25#step:9:37

seems like the only thing that's wrong is that $CONTEXT is not set in test environment. i think if we just pass this in the test case we're golden

@tweak-wtf tweak-wtf merged commit 30f9f9f into develop Sep 14, 2024
2 checks passed
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.

3 participants