Accessor.to_dict requires values to be floats

[Phrogz]

Describe the bug
The Accessor class can be created using integer values, intended to be stored as UnsignedByte, but attempting to convert it to JSON fails because of this line calling to_float instead of from_float (or some sort of from_number):

result["max"] = from_union([lambda x: from_list(to_float, x), from_none], self.max)

To Reproduce
The following (simplified) code from an extension is intended to create an animation where the values are boolean values, stored as UnsignedByte. This hook code runs without issue:

def gather_gltf_hook(self, active_scene_idx, scenes, animations, export_settings):
    times = [0.0, 0.5, 1.0]
    values = [0, 1, 0]

    input_accessor = gather_accessor(
        buffer_view=BinaryData.from_list(times, ComponentType.Float),
        component_type=ComponentType.Float,
        count=len(times),
        max=[max(times)],
        min=[min(times)],
        type=DataType.Scalar,
        export_settings=export_settings
    )

    output_accessor = gather_accessor(
        buffer_view=BinaryData.from_list(values, ComponentType.UnsignedByte),
        component_type=ComponentType.UnsignedByte,
        count=len(values),
        max=[max(values)],
        min=[min(values)],
        type=DataType.Scalar,
        export_settings=export_settings
    )

    sampler = AnimationSampler(
        extensions=None,
        extras=None,
        input=input_accessor,
        interpolation="STEP",
        output=output_accessor
    )

    target = AnimationChannelTarget(
        extensions={ "KHR_animation_pointer" : Extension(
            name="KHR_animation_pointer",
            extension={
                "pointer": f"/nodes/0/extensions/EXT_node_visibility/visibility"
            }
        )},
        extras=None,
        node=None,
        path="pointer"
    )

    channel = AnimationChannel(
        extensions=None,
        extras=None,
        sampler=0,
        target=target
    )

    animations.append(Animation(
        name="Object Blink",
        extensions=None,
        channels=[channel],
        samplers=[sampler],
        extras=None,
    ))

However, when the glTF tries to be serialized to include this animation, it errors out with:

  File "/Applications/Blender 4.1.app/Contents/Resources/4.1/scripts/addons/io_scene_gltf2/io/com/gltf2_io.py", line 36, in from_union
    return f(x)
           ^^^^
  File "/Applications/Blender 4.1.app/Contents/Resources/4.1/scripts/addons/io_scene_gltf2/io/com/gltf2_io.py", line 255, in <lambda>
    result["max"] = from_union([lambda x: from_list(to_float, x), from_none], self.max)
                                          ^^^^^^^^^^^^^^^^^^^^^^
  File "/Applications/Blender 4.1.app/Contents/Resources/4.1/scripts/addons/io_scene_gltf2/io/com/gltf2_io.py", line 61, in from_list
    return [f(y) for y in x]
           ^^^^^^^^^^^^^^^^^
  File "/Applications/Blender 4.1.app/Contents/Resources/4.1/scripts/addons/io_scene_gltf2/io/com/gltf2_io.py", line 61, in <listcomp>
    return [f(y) for y in x]
            ^^^^
  File "/Applications/Blender 4.1.app/Contents/Resources/4.1/scripts/addons/io_scene_gltf2/io/com/gltf2_io.py", line 80, in to_float
    assert isinstance(x, float)
01:57:36 | ERROR: An error occurred on line 36 in statement return f(x)
01:57:36 | ERROR: An error occurred on line 255 in statement result["max"] = from_union([lambda x: from_list(to_float, x), from_none], self.max)
01:57:36 | ERROR: An error occurred on line 61 in statement return [f(y) for y in x]
01:57:36 | ERROR: An error occurred on line 61 in statement return [f(y) for y in x]
01:57:36 | ERROR: An error occurred on line 80 in statement assert isinstance(x, float)

Changing the values from [0, 1, 0] to [0.0, 1.0, 0.0] and the component type to Float fixes the problem, but stores undesired data.

Expected behavior
An accessor for integer values should be possible.

Version
macOS 4.1.1

[Phrogz]

Note that the from_dict() method uses the from_float() function for these same fields, which accepts integers and coerces them to floats...

max = from_union([lambda x: from_list(from_float, x), from_none], obj.get("max"))

def from_float(x):
    assert isinstance(x, (float, int)) and not isinstance(x, bool)
    return float(x)

…whereas the to_dict() method uses the to_float() method, which—despite its name—does not attempt to coerce any value to a float, but just errors out if it's not passed a float:

result["max"] = from_union([lambda x: from_list(to_float, x), from_none], self.max)

def to_float(x):
    assert isinstance(x, float)
    return x

[julienduroure]

Hello,

There are many issue in your code :

• For input_accessor, see Accessor class to accept single values for max and min #2335
• For output_accessor, min/max are optional, you can set them to None for now, as a workaround
• You should not use named parameter for cached function, as it will break the cache system in some cases
• Your Animation constructor is not right

Here is a working code example

    def gather_gltf_hook(self, active_scene_idx, scenes, animations, export_settings):
        from io_scene_gltf2.io.com.gltf2_io_extensions import Extension
        from io_scene_gltf2.blender.exp.accessors import gather_accessor
        from io_scene_gltf2.io.exp.binary_data import BinaryData
        from io_scene_gltf2.io.com.constants import ComponentType, DataType
        from io_scene_gltf2.io.com.gltf2_io import Animation, AnimationChannel, AnimationChannelTarget, AnimationSampler
        times = [0.0, 0.5, 1.0]
        values = [0, 1, 0]

        input_accessor = gather_accessor(
            BinaryData.from_list(times, ComponentType.Float),
            ComponentType.Float,
            len(times),
            tuple([max(times)]),
            tuple([min(times)]),
            DataType.Scalar,
            export_settings
        )

        output_accessor = gather_accessor(
            BinaryData.from_list(values, ComponentType.UnsignedByte),
            ComponentType.UnsignedByte,
            len(values),
            tuple([max(values)]),
            tuple([min(values)]),
            DataType.Scalar,
            export_settings
        )

        sampler = AnimationSampler(
            None,
            None,
            input_accessor,
            "STEP",
            output_accessor
        )

        target = AnimationChannelTarget(
            { "KHR_animation_pointer" : Extension(
                name="KHR_animation_pointer",
                extension={
                    "pointer": f"/nodes/0/extensions/EXT_node_visibility/visibility"
                }
            )},
            None,
            None,
            "pointer"
        )

        channel = AnimationChannel(
            None,
            None,
            0,
            target
        )

        animations.append(Animation(
            [channel],
            None,
            None,
            'Object Blink',
            [sampler]

        ))

For the original bug:
Seems quicktype (that generated the code from jsonschema) does not take into account the multiple type that min/max can have.
This (main...fix_check_number_accessor_min_max) should solve the issue, but I first need to check if there are some regression

[Phrogz]

I very much appreciate your feedback. I'm not new to programming, but I'm new to programming Python, so your feedback about cached functions and keyword arguments is useful. I have two follow-on questions (unrelated to the bug) if you have the time:

1. I'm curious why tuple([n]) is needed as the argument to gather_accessor(), when all it does with it is call list(max), and [n] == list(tuple([n])).

2. I don't see the difference in what you wrote for invoking the Animation constructor compared to what I wrote, other than positional vs. keyword arguments. I'd appreciate any insight you can offer here.

[julienduroure]

The function has a decorator for caching (See https://github.com/KhronosGroup/glTF-Blender-IO/blob/295bbf7eda4c3de5429a01a0d919f42d90aaf5f9/addons/io_scene_gltf2/blender/exp/accessors.py#L22C1-L22C8)

The cache decorator definition is here :

glTF-Blender-IO/addons/io_scene_gltf2/blender/exp/cache.py

Line 55 in 295bbf7

def default_key(*args, **kwargs):

To be able to cache correctly, we have to get the parameter always in the same order, so no named parameter
The same way, for caching, we need to have all parameter to be hashable, so using tuple when needed

[julienduroure]

Fixed by 1dabfa9