Chore: Add more keys for folder template data #362
Conversation
ynbot
added
size/XS
type: enhancement
iLLiCiTiT
requested review from
m-u-r-p-h-y,
simonebarbieri,
tokejepsen,
BigRoy,
mkolar,
kalisp,
MustafaJafar,
antirotor,
jakubjezek001 and
dee-ynput
iLLiCiTiT
changed the title
| @@ -399,6 +399,10 @@ def _fill_folder_data(self, instance, project_entity, anatomy_data): | |||
| "parent": parent_name, | |||
There was a problem hiding this comment.
Shouldn't this just be calling the function from ayon_core.pipeline.template_data.get_folder_template_data?
data = get_folder_template_data(folder_entity)
anatomy_data.update(data)
| "path": path, | ||
| "type": folder_entity["folderType"], | ||
| "parents": hierarchy, | ||
| "parent": parent_name, |
There was a problem hiding this comment.
Is there really use in having the parent data duplicated outside the folder key and inside?
There was a problem hiding this comment.
I would say tha the data which are now in root of dictionary should be considered as deprecated with these changes.
There was a problem hiding this comment.
Just to check I get it correctly.
these data in the screenshot should be considered as deprecated
which means:
- ✅
{root[work]}/{project[name]}/{folder[parents]} - ❌
{root[work]}/{project[name]}/{parents}
| @@ -411,13 +415,18 @@ def _fill_folder_data(self, instance, project_entity, anatomy_data): | |||
| if hierarchy: | |||
| parent_name = hierarchy.split("/")[-1] | |||
|
|
|||
| folder_name = instance.data["folderPath"].split("/")[-1] | |||
| folder_path = instance.data["folderPath"] | |||
| folder_name = folder_path.split("/")[-1] | |||
| anatomy_data.update({ | |||
There was a problem hiding this comment.
If we implement get_folder_template_data in a way that it calls e.g. get_folder_template_data_from_values(folder_path, folder_type, project_name) then also here we could re-use the implementation of ayon_core.template_data.get_folder_template_data_from_values to ensure each of these end up using the same function instead of duplicating the code everywhere that seem to define these template data. Preferably there's one entry point to this. that returns the valid data - instead of having this duplicated.
There was a problem hiding this comment.
Can we focus more on the data keys?
There was a problem hiding this comment.
Might be better then to first fix the duplication of logic in a separate PR which would make the size of this PR change much smaller since there's much less to change.
There was a problem hiding this comment.
I created this PR to raise discussion about the keys....
| "parent": parent_name, | ||
| "parents": hierarchy, |
There was a problem hiding this comment.
I personally think it's confusing that "parent" includes the project name if there are no parents for the folder but parents never includes the project name.
We could keep e.g. hierarchy to be what it was (all parents excluding project) and add parents to be {project_name}/{hierarchy} so that parent makes sense again?
There was a problem hiding this comment.
I do agree that if parent can be project name then parents are confusing. My thinking was that hierarchy should probably contain the folder name too, but maybe you're right.
Ad. "parent" I don't remember why the project name is used? I do remember there was a discussion but don't remember why we chose this approach?
There was a problem hiding this comment.
My thinking was that hierarchy should probably contain the folder name too, but maybe you're right.
Kind of had the same - but by having {hierarchy} separate from the folder name itself you can actually use them separately, plus if they were combined then you'd just have folder[path]? :)
There was a problem hiding this comment.
I agree on having hierarchy as "hierarchy within the project" - so without project name and without folder name. parents should be basically dirname(folder[path]) and parent direct ancestor of the folder?
There was a problem hiding this comment.
This is cool it works on my side. I used it with this Houdini feature that uses template data.
I think we should update Available template keys
|
Is there matching update of documentation (most likely https://ayon.ynput.io/docs/admin_settings_project_anatomy/#available-template-keys)? |
I am guessing that once the proposal will turn into implementation then yes it will need to have that step. |
|
Created PR adding |
|
Closing. Keys |
Changelog Description
Proposal of new keys available in folder dictionary.
Additional info
Added new keys under folder template data. First obvious is
"type"where is folder type. Key"path"is just a folder path,"parent"is name of parent which is project if direct parent is folder,"parents"is what"hirarchy"was.Any comments, ideas, confrontations welcommed.