CreatedInstance `data` argument ambiguity

[antirotor]

Is there an existing issue for this?

• I have searched the existing issues.

Please describe the feature you have in mind and explain what the current shortcomings are?

Problem

CreateInstance is a product of creator plugins. To construct it, you need to pass data argument that is currently typed as dict[str, Any] yet there is some logic actually expecting something specific inside.

ayon-core/client/ayon_core/pipeline/create/structures.py

Lines 490 to 492 in 0bcc26d

	data: Dict[str, Any],
	creator: "BaseCreator",
	transient_data: Optional[Dict[str, Any]] = None,

We shouldn't use these super-generic and non-descriptive arguments (along with the transient_data that is another argument there).

How would you imagine the implementation of the feature?

This should be probably handled by dataclass or some other clearly defined and typed approach. If this is not enough, we should split CreatedInstance into more blocks.

Are there any labels you wish to add?

• I have added the relevant labels to the enhancement request.

Describe alternatives you've considered:

No response

Additional context:

No response

[BigRoy]

I've had some struggles there before. But, isn't the whole idea that you create these instances, either via:

• CreateContext.create to create new ones,
• or CreatedInstance.from_existing(data, self) from existing persisted data in the scene?

So, really - aside of you just the CreateContext data needing to know what data to prep we're not really in dire need to manage it elsewhere? (Although it'd still be good to know what that data should require, etc... but what's the need to fiddle with that data in a different way in your use case?)

Also, transient_data is completely arbitrary data and is up to the Creator to define what to put in their (if it needs any data). CreatedInstance itself does not require any data there in particular. (In a sense it suffers from similar issues like pyblish in that any Creator can store it's own data.)

Anyway, what's your use case? :)

[munkybutt]

TypedDict is a good solution to provide the signature for this (and other dicts with non-arbitrary structures):
https://peps.python.org/pep-0589/
I add signatures locally using these as it is a good compromise vs refactoring huge portions of the API, though dataclasses would ultimately be cleaner.

[antirotor]

Anyway, what's your use case? :)

apart of just having gut feeling that it is just wrong, there are few things to consider:

• quality of life when developing - type hints and documentation - there is no easy way to do that now
• there is some structure expected, like creator_attributes, publish_attributes, productType, productName, it is also adding stuff to data so it is not just something to pass. Any changes in that can break stuff and we'll never know until it is too late.
• tests - difficult to write test if you really don't know what to pass - what needs to be there at all costs, what will change the logic if present, what will have no effect at all.

It has snowball efect - even now, you hava data and inside creator_attributes. What are they? How they are defined? Is there clean API to define them?

I think we need to split the responsibility to more pieces. Dataclasses can help there (or as @munkybutt suggested TypedDict at least until dataclasses can be established). Same goes for all the uses of *args and **kwargs across the whole API. So hard to document (and use then).

Back to the use-case:

I've added new key there product_base_type in #1306 and it broke Maya because of it's default value was None (ynput/ayon-maya#282).

[BigRoy]

TypedDict is a good solution to provide the signature for this (and other dicts with non-arbitrary structures):
peps.python.org/pep-0589

Just wanted to note that I love TypedDict for some of these use cases, but do note that it was added in Python 3.8 and we still need to support Python 3.7 :(

[munkybutt]

TypedDict is a good solution to provide the signature for this (and other dicts with non-arbitrary structures):
peps.python.org/pep-0589

Just wanted to note that I love TypedDict for some of these use cases, but do note that it was added in Python 3.8 and we still need to support Python 3.7 :(

I think TypedDict is backported to typing_extensions?
Also they can be declared in a backwards compatible fashion:
https://peps.python.org/pep-0589/#alternative-syntax

It is also possible to wrap the type declaration in a non-importable block:

from __future__ import annotations  # <- required for delayed evaluation of types (or declare them as strings)

_typing = False
if _typing:
    # this will never evaluate:
    T = TypedDict("T", {"key": value})


# but can be used as type annotations:
def test(arg: T) -> T:
   pass

I love how clean the Ayon codebase is, I don't love the lack of type annotations - especially for critical dictionaries that can very easily break stuff.

[antirotor]

... and we still need to support Python 3.7 :(

I think we won't be able for much longer. Plan is to switch to 3.11 and I am afraid with that switch we'll probably end support for 3.7

[munkybutt]

... and we still need to support Python 3.7 :(

I think we won't be able for much longer. Plan is to switch to 3.11 and I am afraid with that switch we'll probably end support for 3.7

that is great news!