AcademySoftwareFoundation / AcademySoftwareFoundation/OpenColorIO

[python][feature-request]`GradingPrimary` change set GradingStyle behavior

Open
#1,644 3 comments 0 reactions 0 assignees View on GitHub
Needs Discussion
Dominant language
C++
Stars
2.1k
Forks
503
PR merge metrics
No merged PRs in 30d

Description

Hello,

When looking at the `GradingPrimary` class, I have a hard time understanding why do we need to specify the _GradingStyle_ in the `__init__` of the class ? I do understand it's kind of needed for its `validate()` method, but then if we ask the _GradingStyle_ in the validate method, why asking it at `__init__` ?
Furthermore we can't get back this value _GradingStyle_ value from the instance so we have to track it before in the code. Which is annoying when we then need in `GradingPrimaryTransform` to specify the _GradingStyle_ AND a `GradingPrimary` instance which already specify a _GradingStyle_.

Consider the following snippet :
```python

gp_lin = ocio.GradingPrimary(ocio.GRADING_LIN)
gp_lin.exposure = 0.5 #need GradingRGBM but let's ignore for the example

gp_log = ocio.GradingPrimary(ocio.GRADING_LOG)
gp_log.brightness = 0.5

gp_list = (gp_lin , gp_log)
for gp in gp_list:
gp_tsfm = ocio.GradingPrimaryTransform(
gp,
gp.?????, # we can't know which style to use
False,
)
...
```

So in my opinion, here is 2 suggestions for how I would see the class behave :

### 1. Style agnostic
you can check compatibility with `validate()`
the user determine at any moment which style the GradingPrimary will be used with,

```python
gp1 = ocio.GradingPrimary()
gp1.exposure = 0.5

gp2 = ocio.GradingPrimary()
gp2.brightness = 0.5
gp2.validate(ocio.GRADING_LOG) # pass fine
gp2.exposure = 0.5
gp2.validate(ocio.GRADING_LOG) # raise a warning/error ?

# the user determine at any moment which style the GradingPrimary will be used with
gp_list = (
(gp1, ocio.GRADING_LIN),
(gp2, ocio.GRADING_LOG)
)
for gpdata in gp_list:
gp_tsfm = ocio.GradingPrimaryTransform(
gpdata[0],
gpdata[1],
False,
)
...
```

### 2. Style bound to instance, flexible

```python
gp1 = ocio.GradingPrimary(ocio.GRADING_LIN)
gp1.exposure = 0.5

gp2 = ocio.GradingPrimary(ocio.GRADING_LOG)
gp2.brightness = 0.5
gp2.validate() # pass fine
gp2.exposure = 0.5
gp2.validate() # raise a warning/error ?

gp_list = (gp1,gp2)
for gp in gp_list:
gp_tsfm = ocio.GradingPrimaryTransform(
gp,
gp.gradingStyle, # new attribute
False,
)
...
```

### 2.2. Style bound to instance, strict

Remove the `validate()` method and perform check on attribute set.

```python
gp1 = ocio.GradingPrimary(ocio.GRADING_LIN)
gp1.exposure = 0.5

gp2 = ocio.GradingPrimary(ocio.GRADING_LOG)
gp2.brightness = 0.5
gp2.exposure = 0.5 # will raise error
gp2.contrast = 0.0 # will raise error

gp_list = (gp1,gp2)
for gp in gp_list:
gp_tsfm = ocio.GradingPrimaryTransform(
gp,
gp.gradingStyle, # new attribute
False,
)
...
```

I don't know if this makes sense or is too subjective.

Cheers.
Liam.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.