ayon-core icon indicating copy to clipboard operation
ayon-core copied to clipboard

Chore: Add 'parents' to folder template data

Open iLLiCiTiT opened this issue 1 year ago • 7 comments

Changelog Description

Add "parents" subkey to folder template data.

Additional info

This does allow to use indexing of parent names in templates. I'm not sure if this is correct approach, but it is the best I could think of. This change does not consider that someone would like to use e.g. labels instead of names in future, hopefully that won't happen.

There is no safeguard for using the list, so if the parent index is not available it will just crash. That might cause issues in specific logic which is filling template paths and does not count on this.

Testing notes:

  1. You can use e.g. {folder[parents[0]]} in templates.
  2. Test it with an optional template key <{folder[parents[0]}> works from a ROOT folder (does not have any parent folders).

iLLiCiTiT avatar Jul 15 '24 13:07 iLLiCiTiT

This does allow to use indexing of parent names in templates. I'm not sure if this is correct approach, but it is the best I could think of. This change does not consider that someone would like to use e.g. labels instead of names in future, hopefully that won't happen.

There is no safeguard for using the list, so if the parent index is not available it will just crash. That might cause issues in specific logic which is filling template paths and does not count on this.

Hmm - yeah, hmm.. this is bound to give issues. If anyone would use e.g. parents[-1] in a template and they are working in an asset that has no parents the template formatting would already fail. This seems very prone to production issues.

Can we re-iterate why parents is needed as a key and what the admin/user is expected to do with it?

BigRoy avatar Jul 15 '24 14:07 BigRoy

asset that has no parents the template formatting would already fail. This seems very prone to production issues.

I wonder if we could use for that reason optional decoration ...}</{folder[parents[-1]]}>/...

jakubjezek001 avatar Jul 16 '24 08:07 jakubjezek001

BTW this seems to be very good case for unittesting.

jakubjezek001 avatar Jul 16 '24 09:07 jakubjezek001

asset that has no parents the template formatting would already fail. This seems very prone to production issues.

I wonder if we could use for that reason optional decoration ...}</{folder[parents[-1]]}>/...

Ah - if that works I must admit that's fine then I suppose.

BigRoy avatar Jul 16 '24 09:07 BigRoy

So we should add to the testing notes:

  • Test it with an optional template key <{folder[parents[0]}> and make sure it works from a ROOT folder (no parent folders)?

BigRoy avatar Jul 29 '24 09:07 BigRoy

Something I didn't think of is output of used values which right now in StringTemplates always outputs dictionary, that has to be changed to support list.

Another issue might be negative index {folder[parents][-1]} would not work either as python formatting resolves -1 as string instead of number.

iLLiCiTiT avatar Jul 31 '24 16:07 iLLiCiTiT

Now it does work as expected. It is possible to use lists in values.

But negative indexes on list can cause issues when the tempalte is used out of StringTemplate so the missing piece is to change the template in result to use modified template in output {parents[-1]} should be in result template as {parents[0]} (or different index based on used values).

Or not? Not sure, at the end if anyone is willing to use the template afterwards he still have to use StringTemplate because of optional keys.

iLLiCiTiT avatar Nov 21 '24 17:11 iLLiCiTiT

Or not? Not sure, at the end if anyone is willing to use the template afterwards he still have to use StringTemplate because of optional keys.

I don't actually think we state anywhere that templates are compatible with python formatting. So if we say that -1 isn't possible, then all is fine by me.

Auto-converting it to proper index is ofc the way too.

If I would have to choose, I'd say -> If we have interactive validator for templates in the Settings, then not supporting -1 is perfectly fine (because you'll know right away). But since we don't, conversion is better?

antirotor avatar Dec 04 '24 13:12 antirotor

I've just used this change in my anatomy template for the default file name: using {folder[parents][1]}.

On disk everything is correct but I'm now getting an error when publishing out of resolve at the Integrate Asset stage:

DEBUG: Anatomy template name: default
CRITICAL: Error when registering
Traceback (most recent call last):
  File "C:\Users\danwn\AppData\Local\Ynput\AYON\addons\core_1.0.14\ayon_core\plugins\publish\integrate.py", line 158, in process
    self.register(instance, file_transactions, filtered_repres)
  File "C:\Users\danwn\AppData\Local\Ynput\AYON\addons\core_1.0.14\ayon_core\plugins\publish\integrate.py", line 246, in register
    prepared = self.prepare_representation(
  File "C:\Users\danwn\AppData\Local\Ynput\AYON\addons\core_1.0.14\ayon_core\plugins\publish\integrate.py", line 777, in prepare_representation
    template_filled = path_template_obj.format_strict(template_data)
  File "C:\Users\danwn\AppData\Local\Ynput\AYON\addons\core_1.0.14\ayon_core\lib\path_templates.py", line 147, in format_strict
    result = self.format(data)
  File "C:\Users\danwn\AppData\Local\Ynput\AYON\addons\core_1.0.14\ayon_core\pipeline\anatomy\templates.py", line 96, in format
    result = StringTemplate.format(self, data)
  File "C:\Users\danwn\AppData\Local\Ynput\AYON\addons\core_1.0.14\ayon_core\lib\path_templates.py", line 125, in format
    part.format(data, result)
  File "C:\Users\danwn\AppData\Local\Ynput\AYON\addons\core_1.0.14\ayon_core\lib\path_templates.py", line 709, in format
    part.format(data, new_result)
  File "C:\Users\danwn\AppData\Local\Ynput\AYON\addons\core_1.0.14\ayon_core\lib\path_templates.py", line 601, in format
    value = value[sub_key]
IndexError: list index out of range

dan-of-dan avatar Feb 03 '25 17:02 dan-of-dan

It could be because folder used for instance does not have available index you require by template. So if template requires {folder[parents][1]} but used folder path is /assets/Bob then parents = ["assets"] so index 1 is out of bounds. You can use optional keys <{folder[parents][1]_> in the template, which means that the key is skipped if the key is not available.

iLLiCiTiT avatar Feb 03 '25 17:02 iLLiCiTiT

It could be because folder used for instance does not have available index you require by template. So if template requires {folder[parents][1]} but used folder path is /assets/Bob then parents = ["assets"] so index 1 is out of bounds. You can use optional keys <{folder[parents][1]_> in the template, which means that the key is skipped if the key is not available.

I did try those optional keys but I still get the same error. My structure is <project_root>/shots/101/100/010

shots = folder 101 = episode (what I'm fetching with {folder[parents][1]) 100 = sequence 010 = shot

Filename template:

{project[code]}<_{folder[parents][1]}>_{parent}_{folder[name]}_{product[name]}_{@version}<_{output}><.{@frame}><_{udim}>.{ext}

dan-of-dan avatar Feb 03 '25 17:02 dan-of-dan

Hmm, discovered a bug. Will be fixed with https://github.com/ynput/ayon-core/pull/1121 .

iLLiCiTiT avatar Feb 04 '25 09:02 iLLiCiTiT

Hmm, discovered a bug. Will be fixed with #1121 .

Thanks for confirming, will keep an eye on the ticket

dan-of-dan avatar Feb 04 '25 09:02 dan-of-dan