-
Notifications
You must be signed in to change notification settings - Fork 49
Integrator for Trait based representations #1147
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
…-basic-trait-type-using-dataclasses
added few helper methods to query/set/remove bunch of traits at once
…-basic-trait-type-using-dataclasses
…-dataclasses" and "origin/develop"
…into feature/911-new-traits-based-integrator
note that this needs `pytest-ayon` dependency to run that will be added in subsequent commits
also added versionless trait id processing and trait validation
also added versionless trait id processing and trait validation
…-basic-trait-type-using-dataclasses
…ype-using-dataclasses' into feature/909-define-basic-trait-type-using-dataclasses
…into feature/911-new-traits-based-integrator
Co-authored-by: Roy Nieterau <[email protected]>
…ype-using-dataclasses' into feature/909-define-basic-trait-type-using-dataclasses
…into feature/911-new-traits-based-integrator
sync traits declared in #909
btw tests are failing because they need changes to be implemented here https://github.com/ynput/pytest-ayon/tree/chore/align-dependencies |
…egrator' into feature/911-new-traits-based-integrator
this is to ensure test will still run on GH.
…egrator' into feature/911-new-traits-based-integrator
this will be at the end added by another PR
…aits-based-integrator
@@ -24,7 +35,7 @@ class AYONInterface(metaclass=_AYONInterfaceMeta): | |||
in the interface. By default, interface does not have any abstract parts. | |||
""" | |||
|
|||
pass | |||
log = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
log = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is actually helping with mypy and hints - ITrayAddon
is using self.log.warning()
but it is not defined there.
Co-authored-by: Jakub Trllo <[email protected]>
Co-authored-by: Jakub Trllo <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the call that demo'ed this PR.
Use traits during publishing to improve data flow (and clarity on how publishing works)
I want to emphasize once more that I think where we would currently gain the most of traits (or at least structured dataflow) is during publishing. Being able to know what kind of data I can or should add during publishing (and before publishing/integrating) is what we're currently struggling with the most. Less so with the "loading" side of things after the data is ingested (because usually that loading part is just easier overall).
As such, I'd LOVE to see more examples of using the traits to actually improve the publishing process itself.
Here I'd love to see:
- How multiple plug-ins would easily build a representation with its traits? This is where we want to avoid storing into some magical
instance.data["myRepresentations"]
data structure that is undefined and instead rely on higher level API functions - this is exactly the part of the publishing that needs streamlining/improving. - How to find the right representation to act upon (e.g. Extract Review).
Because the loading logic is usually relatively trivial I'm actually more worried that "traits" would solely make them more complicated.
Preferably, the data flow improvements also allow streamlining (or offloading to the farm) e.g. the full review process. So that all we'd do I store the "Traits" of data, for then another farm job to act upon. Just so we have structured data to act on, instead of instance.data
with ANY untyped data.
Backwards compatibility
With "Loaders" intended to start relying on Traits data that'd also mean that those loader logics wouldn't be backwards compatible with existing publishes. Meaning that those loaders wouldn't be valid for AYON deployments for years to come to remain backwards compatible. Unless we either implement fallbacks in the code (which means suddenly we now need to maintain BOTH the old and new logic on loaders, for a very long time OR find a way to automatically apply traits to existing publishes.)
If we want our loaders to use Traits data from the published representations I think the only way forward is to automatically compute trait data for existing publishes. Because maintaing BOTH code logic in Loaders forever (since loading should be VERY backwards compatible, as much as we can, forever) seems to just double the workload to no benefit but only adding complexity.
Some other questions:
- How do I represent an animated UDIM sequence? Currently it seems a sequence is either UDIM or not.
- How does traits work with a Maya look publish that generates a
.ma
file, a .json
and ANY amount of resources? I assume each "resource" needs to be tracked now with traits (does that mean that these files MUST now become representations each?) - With FrameRanged trait, what is
in
andout
versusstart
andend
? I have no idea.
Some notes:
- I feel like
KeepOriginalName
andKeepOriginalLocation
isn't really a "trait" of the published data. I feel the trait is named wrong maybe? Maybe it should beUsesOriginalName
? But even then, is this really a "trait" of the published data? or just a rule during ingesting/integrating? I think these are somehow off in what traits are supposed to be?
|
||
@dataclass | ||
class Lighting(TraitBase): | ||
"""Lighting trait model. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be me - but not entirely sure what "Lighting" trait would apply to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, this one serves more like an example of type trait. Maybe superceded by IntendedUse? Imagine compound representation using Bundle publishing fbx with lights, IES profiles in separate file, etc. Might be used to tag "generic" fbx that it contains Lighting? It is overlapping with the product itself, for sure.
frame_end: int | ||
frame_in: Optional[int] = None | ||
frame_out: Optional[int] = None | ||
frames_per_second: str = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FPS should likely be a separate trait, no?
Also, what happens if something would have a variable frame rate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also I am not sure why is fps in string and not float?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is part of the original OpenAssetIO trait, that is why it is there. If variable frame rate is ever needed then yes, it should be separate trait and variable frame rate can be described by another trait with lists? Dunno
|
||
name: ClassVar[str] = "Transient" | ||
description: ClassVar[str] = "Transient Trait Model" | ||
id: ClassVar[str] = "ayon.lifecycle.Transient.v1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If a new version of this trait would get created, does that mean we'd need a different class? Or how would we actually version traits in practice?
I assume somehow we'd need to keep the old trait class implementation around?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Issue is that versioning make sense in OTIO schemas or the original OAIO where we deal with json schema files, right? This way we will need to add version even to the class name, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whenever you want to de-serialize data that are using older version of trait, upgrade()
method on newer trait definition is called to reconstruct new version (downgrading isn't possible). That means when you encounter trait like ayon.2d.Image.v1
in data and your runtime Image trait is v2, some method will call upgrade()
method on newer trait version if implemented (for example in cases where newer version knows how to provide missing data or do conversion, etc.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For reference - upgrade()
is called with old trait as an argument on .from_dict()
calls and when you get traits by id and you omit version specifier (and old trait different than the runtime version is found)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have tested it as it is and nothing had been broken
Thanks for your points and questions. Regarding
I see representation traits as the lowest level building block for this. Once you can describe the results of the publishing, you can build API and structures on top of it. For example - you could have
class ProcessDiffuseTexture(pyblish.api.InstancePlugin):
...
def process(self, instance: pyblish.api.Instance) -> None:
...
diffuse_texture_traits = [
Image(),
PixelBased(
display_window_width =1920,
display_window_height = 1080,
pixel_aspect_ratio = 1.0
)]
add_trait_representation(instance, Representation("foo", diffuse_texture_traits))
... Then plugin to fill in lets say planar configuration: class CollectTextiureConfiguration(pyblish.api.InstancePlugin):
...
def process(self, instance: pyblish.api.Instance) -> None:
...
# add planar configuration to all Image representation that are missing it.
for repre in get_trait_representations(instance):
if repre.has_trait(Image) and not repre.has_trait(Planar):
repre.add_trait(Planar(planar_configuration="RGB"))
... After that you want to modify pixel aspect just on representation class ChangeFooPAR(pyblish.api.InstancePlugin):
...
def process(self, instance: pyblish.api.Instance) -> None:
...
repre = next(rep for rep in get_trait_representations(instance) if rep.name == "foo")
if repre and repre.has_trait(PixelBased):
repre[PixelBased.id].pixel_aspect_ratio = 1.0 We might have helper function
So far the traits can help where there is ambiguity - where loaders can't do better than guess. Then we can introduce new class of loaders that will just work with trait based representation. And if that is not an option, loader with fallback logic if traits are not present. I think in most cases the loaders will use traits if there is no other way around or in cases where you need to load compound type representations - then you can have one loader using traits (because there is no other way) invoking old-style loaders on subrepresentations for example. But that is per-use case. I agree that dropping backwards compatibility in loader for traits needlessly isn't really an option.
Honestly not sure right now. Something to find out definitely.
You are right that everything needs to be tracked. This is necessary for better site sync functionality etc. anyway. Resources are logically bound to some representation - i.e. they are needed for working with loaded product - like the look you've mentioned. In this case you would use Bundle trait as follows: maya_file = [
FileLocation(file_path=Path("/path/to/look/shaders.ma")),
MimeType(mime_type="application/maya"), # or whatever?
Tagged(tags=["maya", "shaders"])
]
relations = [
FileLocation(file_path=Path("/path/to/look/relations.json")),
MimeType(mime_type="text/json"),
SourceApplication(
application="Maya",
version="2026",
platform="Windows x86_64")
]
diffuse_texture = [
Image(),
PixelBased(
display_window_width=1920,
display_window_height=1080,
pixel_aspect_ratio=1.0),
Planar(planar_configuration="RGB"),
FileLocation(
file_path=Path("/path/to/diffuse.jpg"),
file_size=1024,
file_hash=None),
MimeType(mime_type="image/jpeg"),
]
bump_texture = [
Image(),
PixelBased(
display_window_width=1920,
display_window_height=1080,
pixel_aspect_ratio=1.0),
Planar(planar_configuration="RGB"),
FileLocation(
file_path=Path("/path/to/bump.tif"),
file_size=1024,
file_hash=None),
MimeType(mime_type="image/tiff"),
]
bundle = Bundle(items=[maya_file, relations, diffuse_texture, bump_texture])
representation = Representation(name="look", traits=[bundle]) This will create single representation with all those files. If you want to create more loose relationship, you can create individual representations and then in main one use Fragment trait that is taking id of the other representations.
This is taken from OpenAssetIO FrameRanged trait and are in essence inclusive handles. It is duplicating Handles trait so up to the debate whether or not to remove it (or remove Handles trait).
Admittedly this is primarily used to drive integration but also keeping the information that the original name and locations were used and not modified by integrator itself. Might be useful for archival tool for example. Naming is difficult as usual, I stayed with what we are already using but I am definitely open to suggestions. |
…egrator' into feature/911-new-traits-based-integrator
Changelog Description
Integrator plugin that will integrate all trait based representations based on current generic integrator.
Note
This already includes #979
Warning
The loader part needs ynput/ayon-python-api#225 and ynput/ayon-backend#552 - from the server side, you need at least 1.7.5
Representations and traits
Introduction
The Representation is the lowest level entity, describing the concrete data chunk that
pipeline can act on. It can be specific file or just a set of metadata. Idea is that one
product version can have multiple representations - Image product can be jpeg or tiff, both formats are representation of the same source.
Brief look into the past (and current state)
So far, representation was defined as dict-like structure:
This is minimal form, but it can have additional keys like
frameStart
,fps
,resolutionWidth
, and more. Thare is alsotags
key that can holdreview
,thumbnail
,delete
,toScanline
and other tag that are controlling the processing.This will be "translated" to similar structure in database:
There are also some assumptions and limitations - like that if
files
in therepresentation are list they need to be sequence of files (it can't be a bunch of
unrelated files).
This system is very flexible in one way, but it lacks few very important things:
unforeseeable
consequences
belong together
it is very expensive (like axis orientation and units from alembic files)
New Representation model
The idea about new representation model is obviously around solving points mentioned
above and also adding some benefits, like consistent IDE hints, typing, built-in
validators and much more.
Design
The new representation is "just" a dictionary of traits. Trait can be anything provided
it is based on
TraitBase
. It shouldn't really duplicate information that isavailable in a moment of loading (or any usage) by other means. It should contain
information that couldn't be determined by the file, or the AYON context. Some of
those traits are aligned with OpenAssetIO Media Creation with hopes of maintained compatibility (it
should be easy enough to convert between OpenAssetIO Traits and AYON Traits).
Details: Representation
Representation
has methods to deal with adding, removing, gettingtraits. It has all the usual stuff like
get_trait()
,add_trait()
,remove_trait()
, etc. But it also has plural forms so you can get/setseveral traits at the same time with
get_traits()
and so on.Representation
also behaves like dictionary. so you can access/settraits in the same way as you would do with dict:
Note
Trait and their ids - every Trait has its id as a string with
version appended - so Image has
ayon.2d.Image.v1
. This is used onseveral places (you see its use above for indexing traits). When querying,
you can also omit the version at the end, and it will try its best to find
the latest possible version. More on that in Traits
You can construct the
Representation
from dictionary (for exampleserialized as JSON) using
Representation.from_dict()
, or you canserialize
Representation
to dict to store withRepresentation.traits_as_dict()
.Every time representation is created, new id is generated. You can pass existing
id when creating new representation instance.
Equality
Two Representations are equal if:
Validation
Representation has
validate()
method that will runvalidate()
onall it's traits.
Details: Traits
As mentioned there are several traits defined directly in ayon-core. They are namespaced
to different packages based on their use:
FileLocation
Traits are Python data classes with optional
validation and helper methods. If they implement
TraitBase.validate(Representation)
method, they can validate against all other traitsin the representation if needed.
Note
They could be easily converted to Pydantic models but since this must run in diverse Python environments inside DCC, we cannot
easily resolve pydantic-core dependency (as it is binary written in Rust).
Note
Every trait has id, name and some human-readable description. Every trait
also has
persistent
property that is by default set to True. ThisControls whether this trait should be stored with the persistent representation
or not. Useful for traits to be used just to control the publishing process.
Examples
Create simple image representation to be integrated by AYON:
To work with the resolution of such representation:
Accessing non-existent traits will result in exception. To test if
representation has some specific trait, you can use
.contains_trait()
method.You can also prepare the whole representation data as a dict and
create it from it:
Addon specific traits
Addon can define its own traits. To do so, it needs to implement
ITraits
interface:Usage in Loaders
In loaders, you can implement
is_compatible_loader()
method to check if therepresentation is compatible with the loader. You can use
Representation.from_dict()
tocreate the representation from the context. You can also use
Representation.contains_traits()
to check if the representation contains the required traits. You can even check for specific
values in the traits.
You can use similar concepts directly in the
load()
method to get the traits. Here isan example of how to use the traits in the hypothetical Maya loader:
Usage Publishing plugins
You can create the representations in the same way as mentioned in the examples above.
Straightforward way is to use
Representation
class and add the traits to it. Collecttraits in list and then pass them to the
Representation
constructor. You should addthe new Representation to the instance data using
add_trait_representations()
function.Developer notes
Adding new trait based representations in to publish Instance and working with them is using
set of helper function defined in
ayon_core.pipeline.publish
module. These are:And their main purpose is to handle the key under which the representation
is stored in the instance data. This is done to avoid name clashes with
other representations. The key is defined in the
AYON_PUBLISH_REPRESENTATION_KEY
.It is strongly recommended to use those functions instead of
directly accessing the instance data. This is to ensure that the
code will work even if the key is changed in the future.
Closes #911