Skip to content

Conversation

@antirotor
Copy link
Member

Changelog Description

When imprinting, skip values that are None as there is no real way to convert them to Maya-native values, nor need for it.

Additional review information

When Maya enounters None value when imprinting, it crashes with TypeError: Unsupported type: <class 'NoneType'>. I think that we can safely assume that if it is None, we don't need to store it.

Testing notes:

  • add some None value to instance data in a creator
  • create instance in Maya using this creator

When imprinting, skip values that are `None` as there is no real way to convert them to Maya-native values, nor need for it.
@antirotor antirotor requested a review from BigRoy June 2, 2025 17:03
@antirotor antirotor self-assigned this Jun 2, 2025
@antirotor antirotor added the type: bug Something isn't working label Jun 2, 2025
@BigRoy
Copy link
Contributor

BigRoy commented Jun 3, 2025

I actually think this isn't the correct fix... or at least, it's hiding the actual source issue where we're actually passing None as a value and may be expecting this persists in some way.

The question then becomes - how does the None value appear in the instance creator? :) How do you reproduce this in a production scenario?

Comment on lines +702 to +703
# None is not a valid value for attributes, so we skip it
continue
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say that at the very least this should log a warning with the attribute name that will be skipped because the value is None so that it still stands out what is happening.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we move the check if the value is None right before the callable(value) check?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we move the check if the value is None right before the callable(value) check?

No, because triggering the callable could still generate a value that is None

@antirotor
Copy link
Member Author

antirotor commented Jun 3, 2025

How do you reproduce this in a production scenario?

It is actually very simple. Just add any custom attribute on the creator that is None:

class TestCreator(plugin.MayaCreator)
    identifier = "io.ayon.creators.maya.test"
    label = "Test"
    product_type = "test"
    icon = "cube"
    default_variants = []

    some_great_attribute = None

in this case some_great_attribute will trigger it. Admittedly, we shoud change what is actually imprinted, but that is IMO too deep now to handle. I had to make this change just to continue my work on ynput/ayon-core#1297

I would argue that emitting any warning is now very much useless as this is usually lost in the console output and is of no use for the artist. Developer will check the code and see the comment. Also, it doesn't break any existing functionality because it would be crashing already with None values. Last point - I think handling None values should be done in the code, not on the stored data itself.

@BigRoy
Copy link
Contributor

BigRoy commented Jun 3, 2025

Wait - what... why are all Creator attributes imprinted? That's an issue in itself! Actually, not true - your example works absolutely fine, without this PR.

I can create instances for this just fine:

from ayon_maya.api import plugin


class TestCreator(plugin.MayaCreator):
    identifier = "io.ayon.creators.maya.test"
    label = "Test"
    product_type = "test"
    icon = "cube"
    default_variants = []

    some_great_attribute = None

I can get the error to occur with:

from ayon_maya.api import plugin


class TestCreator(plugin.MayaCreator):
    identifier = "io.ayon.creators.maya.test"
    label = "Test"
    product_type = "test"
    icon = "cube"
    default_variants = []

    def create(self, product_name, instance_data, pre_create_data):
        instance_data["world"] = None
        super().create(product_name, instance_data, pre_create_data)

However:

  1. That's a misuse to begin with?
  2. Plus, if I do want to explicitly set None in instance_data then I DO want it to persist. So the fact that it fails, means we should improve the imprint and read to allow None to be a valid attribute value for the persisted data instead.

@antirotor
Copy link
Member Author

You are right but ... My issues are coming from CreatedInstance class that is defined in ayon-core and is affecting whatever gets imprinted. Any change you do there will eventually break Maya or whatever host with limited ability to store data types. Those hosts using JSON are in better slightly position.

I think we should be able to store some defined set of data types in any host. The questions are:

  • how to define that set (some interface, validation function, ...)
  • how to store None values in Maya if we decide that we need to do so
  • should we store instance data in JSON in Maya?

I realize that first question is more for ayon-core.

@antirotor
Copy link
Member Author

I think it is highly related to this issue I've created on ayon-core: ynput/ayon-core#1304

I believe passing of untyped and unchecked dictionaires like data and transient_data as we are doing with CreatedInstance is the root of this problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants