Settings: Move imageio from project anatomy to project settings #3909

BigRoy · 2022-09-23T19:53:38Z

Brief description

This implements #3865 by moving imageio settings per host from project anatomy to project settings.

Before:

project_anatomy/imageio/{app}

After:

project_settings/{app}/imageio

Description

Currently untested - no backwards compatible or fallbacks implemented. Basically your old project anatomy customized settings will be lost. 🔥 - So as #3865 we'll need to look into how we'll want to transfer those settings over automagically 🪄 .

Additional info

Note that each imageio group has now set is_group to True as such changing one setting in the imageio for a host will now override that full imageio block of settings for the project. It's up for discussion whether we want that - if we want to change around we'll need to add some labels to some of the settings (required for is_group to be False) but that should be fine.

It could be that some documentation might also need to be updated along.

Even though admittedly the old Color Managements docs are very slim - it's just the header - ha!
But potentially a comment on and an update to description of this Nuke Color Management tutorial video would be worthwhile too to make users aware the settings have moved over in the future.
Hiero documentation will need to be updated around apply colorspace project

Copying ImageIO settings from Anatomy to Project Settings

Any potential backwards compatible "updating" would only need to basically 'copy the settings values' from project_anatomy/imageio/{app} on top of project_settings/{app}/imageio to transfer the exact settings. So in essence it is:

for app in anatomy["imageio"]:
    value = app.value
    project_settings[app.key]["imageio"].set(value)
    project_settings.save()

I'm not awaiting the moment this PR gets 'assigned' to me and I get all the blame in the future. 🗡️

Testing notes:

🔥 🚒 Get your fire extinguishers ready
Test color management related features across the hosts

- Note: There is no backwards compatibility implemented

BigRoy · 2022-09-23T19:55:47Z

schema/config-2.0.json

- "imageio": {
- "type": "object"
- },


Can we just remove this? Are old projects ever checked against this schema and invalidated if it finds an entry that isn't in this config?

It's not a required key so that helps, but still.

BigRoy · 2022-09-23T19:56:32Z

openpype/settings/lib.py

-def _project_anatomy_backwards_compatible_conversion(project_anatomy):
- # Backwards compatibility of node settings in Nuke 3.9.x - 3.10.0
- # - source PR - https://github.com/pypeclub/OpenPype/pull/3143
- value = project_anatomy
- for key in ("imageio", "nuke", "nodes", "requiredNodes"):
- if key not in value:
- return
- value = value[key]
-
- for item in value:
- for node in item.get("knobs") or []:
- if "type" in node:
- break
- node["type"] = "__legacy__"
-
-


Had no use with this move of the settings as far as I know - so burned it down to the floor.

#3909 (comment)

iLLiCiTiT · 2022-09-23T21:51:08Z

I would keep schemas for openpype/settings/defaults/project_anatomy/imageio.json so you can copy->paste values on update without loosing them on update.

BigRoy · 2022-09-24T08:10:57Z

I would keep schemas for openpype/settings/defaults/project_anatomy/imageio.json so you can copy->paste values on update without loosing them on update.

Since we are matching the settings 1-to-1 wouldn't it be much nicer if it'd automatically transfer the settings from Project Anatomy somehow? I feel from a User perspective that'd be much nicer than still having to "fix" each project manually (even though sure, you could copy/paste the values - but similarly we could offer a "click here and it's fixed button" by copying the data project anatomy in database to the settings?

I'm mostly worried about having both setting visible and the user misunderstanding that those in Project Anatomy essentially do nothing. We could put in a big deprecation label in the UI, but still. If the intend is for production to notice the least the settings should be copied over thus it sounds like we should just make that happen.

iLLiCiTiT · 2022-09-24T08:20:49Z

I'm mostly worried about having both setting visible and the user misunderstanding that those in Project Anatomy essentially do nothing. We could put in a big deprecation label in the UI, but still. If the intend is for production to notice the least the settings should be copied over thus it sounds like we should just make that happen.

The problem is that you will loose all those data from anatomy on first save of project setting. So if you're "testing" new version in production you will actually break all projects on project settings save.

NOTE: We're first testing OpenPype as staging before full deploy to production.

- This way project save will not delete the old settings

BigRoy · 2022-09-29T09:31:12Z

Question:

Should we maybe swap these around so the settings are

project_settings/imageio/{app}

instead of

project_settings/{app}/imageio?

The benefit I'd see from that is that we can just make one MASTER config setting that sets OCIO env var as a hook based on the config path set in project_settings/imageio/config_path. Potentially still allowing apps to have an explicit project_settings/imageio/app/override_config_path like setting?

Or is it very likely that the different applications should use different OCIO configs in a project.

What do we feel is the most sensible 'project setting'? @mkolar @maxpareschi

Of course this is very related to Colorspace Distribution and Management

iLLiCiTiT · 2022-09-29T09:57:05Z

That is ultimate goal but we can't do it at this moment. Nuke imageio has totally different settings then Maya or Flame or Hiero. We need this PR to move the settings from anatomy to project setting so we can possibly change their behavior from version to version with versioned settings which is not possible if they're in anatomy.

BigRoy · 2022-09-29T10:49:37Z

Great - then I suspect this PR should be ok to start that transition.

jakubjezek001 · 2022-10-11T09:30:08Z

openpype/hosts/nuke/api/lib.py

@@ -563,7 +563,7 @@ def get_node_path(path, padding=4):


 def get_nuke_imageio_settings():
- return get_anatomy_settings(Context.project_name)["imageio"]["nuke"]
+ return get_project_settings(Context.project_name)["nuke"]["imageio"]


I believe there should be some backward compatibility check. Some clients might have Anatomy ImageIO set from 3.9 and they didn't change __legacy__ type yet. We would need to go trough all projects and do this manually (impossible).
For those projects it doesnt even make sense to copy paste it from Anatomy to ./nuke/imageio.

I will do the changes in separate PR. Please could you return the ___legacy__ check (#3909 (comment))

jakubjezek001

good for merge.. tested Nuke, Hiero. Resolve is not done anyway. Flame should be fine, too.

jakubjezek001 · 2022-10-11T10:54:33Z

good for merge.. tested Nuke, Hiero. Resolve is not done anyway. Flame should be fine, too.

huh.. just realized that the __legacy__ (#3909 (comment)) needs to be returned before the merge..

iLLiCiTiT

We need explicit backwards compatibility in project settings. @jakubjezek001 can you do required changes in new branch (sourcing from this) and create new PR?

jakubjezek001 · 2022-10-11T11:15:22Z

This PR is closing in favor of #3959

BigRoy added 2 commits September 23, 2022 21:37

Move imageio settings from project anatomy to project settings

ae5ec70

- Note: There is no backwards compatibility implemented

Remove imageio from config-2.0

843b52c

BigRoy commented Sep 23, 2022

View reviewed changes

BigRoy added 3 commits September 24, 2022 12:03

Revert removal of anatomy imageio settings

4a98d8d

- This way project save will not delete the old settings

Revert imageio removal from project anatomy in config schema

9ff7d56

Add deprecation labels

92371d5

BigRoy marked this pull request as ready for review September 29, 2022 10:49

jakubjezek001 requested review from jakubjezek001 and antirotor October 11, 2022 09:19

jakubjezek001 assigned jakubjezek001, antirotor and BigRoy and unassigned jakubjezek001 and antirotor Oct 11, 2022

jakubjezek001 reviewed Oct 11, 2022

View reviewed changes

jakubjezek001 requested review from jakubjezek001 and iLLiCiTiT October 11, 2022 09:33

jakubjezek001 approved these changes Oct 11, 2022

View reviewed changes

iLLiCiTiT suggested changes Oct 11, 2022

View reviewed changes

jakubjezek001 mentioned this pull request Oct 11, 2022

Settings: Move imageio from project anatomy to project settings [pypeclub] #3959

Merged

6 tasks

jakubjezek001 closed this Oct 11, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Settings: Move imageio from project anatomy to project settings #3909

Settings: Move imageio from project anatomy to project settings #3909

BigRoy commented Sep 23, 2022 •

edited by jakubjezek001

Loading

BigRoy Sep 23, 2022

BigRoy Sep 23, 2022

jakubjezek001 Oct 11, 2022

iLLiCiTiT commented Sep 23, 2022

BigRoy commented Sep 24, 2022

iLLiCiTiT commented Sep 24, 2022 •

edited

Loading

BigRoy commented Sep 29, 2022 •

edited

Loading

iLLiCiTiT commented Sep 29, 2022

BigRoy commented Sep 29, 2022

jakubjezek001 Oct 11, 2022

jakubjezek001 left a comment

jakubjezek001 commented Oct 11, 2022

iLLiCiTiT left a comment

jakubjezek001 commented Oct 11, 2022

Settings: Move imageio from project anatomy to project settings #3909

Settings: Move imageio from project anatomy to project settings #3909

Conversation

BigRoy commented Sep 23, 2022 • edited by jakubjezek001 Loading

Brief description

Description

Additional info

Copying ImageIO settings from Anatomy to Project Settings

Testing notes:

BigRoy Sep 23, 2022

Choose a reason for hiding this comment

BigRoy Sep 23, 2022

Choose a reason for hiding this comment

jakubjezek001 Oct 11, 2022

Choose a reason for hiding this comment

iLLiCiTiT commented Sep 23, 2022

BigRoy commented Sep 24, 2022

iLLiCiTiT commented Sep 24, 2022 • edited Loading

BigRoy commented Sep 29, 2022 • edited Loading

iLLiCiTiT commented Sep 29, 2022

BigRoy commented Sep 29, 2022

jakubjezek001 Oct 11, 2022

Choose a reason for hiding this comment

jakubjezek001 left a comment

Choose a reason for hiding this comment

jakubjezek001 commented Oct 11, 2022

iLLiCiTiT left a comment

Choose a reason for hiding this comment

jakubjezek001 commented Oct 11, 2022

BigRoy commented Sep 23, 2022 •

edited by jakubjezek001

Loading

iLLiCiTiT commented Sep 24, 2022 •

edited

Loading

BigRoy commented Sep 29, 2022 •

edited

Loading