-
Notifications
You must be signed in to change notification settings - Fork 68
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
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 <roy_nieterau@hotmail.com>
…ype-using-dataclasses' into feature/909-define-basic-trait-type-using-dataclasses
…into feature/911-new-traits-based-integrator
sync traits declared in #909
| """ | ||
|
|
||
| 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 <43494761+iLLiCiTiT@users.noreply.github.com>
Co-authored-by: Jakub Trllo <43494761+iLLiCiTiT@users.noreply.github.com>
BigRoy
left a comment
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
.mafile, a .jsonand 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
inandoutversusstartandend? I have no idea.
Some notes:
- I feel like
KeepOriginalNameandKeepOriginalLocationisn'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)
jakubjezek001
left a comment
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.0We 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
|
These are some actionable points from our internal call summarized by AI.
There si no guessing. For example - def get_file_location_for_frame(
self,
frame: int,
sequence_trait: Optional[Sequence] = None,
) -> Optional[FileLocation]:
...This function will return specific file path for a given frame number. Also: frame_padding: int
gaps_policy: Optional[GapPolicy] = GapPolicy.forbidden
frame_regex: Optional[Pattern] = None
frame_spec: Optional[str] = NoneWith those you can describe the sequence completely and they are used in validator functions.
If I am not mistaken, paths are resolved by integrator so they are at the end absolute. Is there any use case for relative paths in the integrated representation traits?
That is certainly possible. I'd say it has merit if there are other types of handles to be described by traits in time domain. I am not sure if there are handles described by timecode for example in use? Any opinions?
FPS attribute is there to align it with OpenAssetIO FrameRanged trait. I see it logical -
Concept of trait IDs and versioning is similar to OpenTimelineIO. Taken from description of #979: You can work with Traits using classes, but you can also utilize their ids. That is useful when working with representation that was serialized into "plain" dictionary: # some pseudo-function to get representation as dict
representation = Representation.from_dict(get_representation_dict(...))
# get trait by its id
# note: the use of version in the ID. You can test if trait is present in representation, or if trait of specific version is present.
if representation.contains_trait_by_id("ayon.content.FileLocation"):
print(f"path: {representation.get_trait_by_id("ayon.content.FileLocation.v1")
# serialize back to dict
representation.traits_as_dict()There is also version upgrade feature. Whenever you want to de-serialize data that are using older version of trait, But your current runtime has an Image trait Whenever you run
Retiming trait is excelent idea and we'll definitely need one. #1313
Transient traits are used for representation-related transient data that can be used for final traits to be integrated with the representation. They are useful mostly for integrators that can take them and transform them to persistent traits. Note that there is also
This was about the auto-conversion of traits of existing data on the instance. Definitely possible for specific cases, but care must be taken about how precise guesses are in the conversion (as some guessing will probably happen anyway).
There is
This is something that will probably deserve its own trait when needed. #1312
Implementation of traits can be (should be) staged. There are some capabilities the Integrator is missing for all workflows and then there are existing key plugins that needs to be changed so they support traits - like Extract Review. This PR is just adding functionality but it is not changing existing one. So I consider this PR as stage 1. Stage 2 is start using traits with data not requiring these key plugins - and at the same time identify what needs to be changed. This will basically define Stage 3 that is about changing behaviour of existing plugins to support traits. Stage 4 is support in loaders. |
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:
{ "name": "foo", "ext": "exr", "files": ["foo_001.exr", "foo_002.exr"], "stagingDir": "/bar/dir" }This is minimal form, but it can have additional keys like
frameStart,fps,resolutionWidth, and more. Thare is alsotagskey that can holdreview,thumbnail,delete,toScanlineand other tag that are controlling the processing.This will be "translated" to similar structure in database:
{ "name": "foo", "version_id": "...", "files": [ { "id": ..., "hash": ..., "name": "foo_001.exr", "path": "{root[work]}/bar/dir/foo_001.exr", "size": 1234, "hash_type": "...", }, ... ], "attrib": { "path": "root/bar/dir/foo_001.exr", "template": "{root[work]}/{project[name]}...", }, "data": { "context": { "ext": "exr", "root": {...}, ... }, "active": True ... }There are also some assumptions and limitations - like that if
filesin 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
Representationhas 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.Representationalso 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
Representationfrom dictionary (for exampleserialized as JSON) using
Representation.from_dict(), or you canserialize
Representationto 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:
FileLocationTraits 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
persistentproperty 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
ITraitsinterface: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
Representationclass and add the traits to it. Collecttraits in list and then pass them to the
Representationconstructor. 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.publishmodule. 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