search

nutti / fake-bpy-module

Fake Blender Python API module collection for the code completion.
MIT License
1.31k stars 95 forks source link

bpy_prop_collection.get() returns None #184

Closed Road-hog123 closed 1 month ago

Road-hog123 commented 5 months ago

System Information

Expected behavior

A method named get() should return a value.

Description about the bug

Mesh.uv_layers is a bpy_prop_collection[MeshUVLoopLayer], so retrieving an element by name with .get() should return either a MeshUVLoopLayer or the type of the default parameter.

typing.Any would be an improvement over None.

I note that foreach_get(), items() and values() could also benefit from GenericType, however get() is complicated by its default parameter.

JonathanPlasse commented 4 months ago

There are existing issues for some of your proposed changes.

nutti commented 1 month ago

I fixed the issues mentioned by @JonathanPlasse.

@Road-hog123 @JonathanPlasse

I'm not sure how to foreach_get(). What is the proper data type for it?

Road-hog123 commented 1 month ago

Mesh.uv_layers is a bpy_prop_collection[MeshUVLoopLayer], so retrieving an element by name with .get() should return either a MeshUVLoopLayer or the type of the default parameter.

As per my comment in #153 the fixed return type of Optional[GenericType] is incorrect when default is not None.

I'm not sure if I should open a separate issue for it, but the key parameters of find() and get() should be str not str | None.

I'm not sure how to foreach_get(). What is the proper data type for it?

foreach_get and foreach_set are problematic because of multi-dimensional array flattening—probably best to open a separate issue for them specifically.

nutti commented 1 month ago

@Road-hog123

foreach_get and foreach_set are problematic because of multi-dimensional array flattening—probably best to open a separate issue for them specifically.

No prob. You can also discuss this here. Before implementing this to our generation code, we need to fix the correct data type for foreach_get and foreach_set at first. Actually, we could not cover all use-cases of this data type, your comments are very useful for us.

Road-hog123 commented 1 month ago

My initial thoughts are: foreach_get(attr: str, seq: MutableSequence) -> None foreach_set(attr: str, seq: Sequence[bool | int | float]) -> None

These are very C-like methods, with foreach_get() trying to avoid memory allocation by writing to an existing sequence—for type-checking purposes I think it just needs to be mutable and support __len__(). As foreach_set() reads from seq it doesn't need to be mutable, but it does still need to support __len__().

The contained type of seq depends on the value of attr—if getattr(GenericType, attr) returns bool then seq should be Sequence[bool], if Vector then it should be Sequence[float] (and the sequence length is multiplied by the number of dimensions the vector has, which doesn't matter for type checking). I don't know if a type checker could actually determine the correct type, but as it will always be one or more of bool, int or float I think a union is a good starting point.

nutti commented 1 month ago

@Road-hog123

I think MutableSequence is now deprecated. Is it possible to use list instead?

Road-hog123 commented 1 month ago

typing.MutableSequence and collections.abc.ByteString are deprecated, but collections.abc.MutableSequence shouldn't be?

nutti commented 1 month ago

@Road-hog123 @JonathanPlasse

Could you give me an advice about this type annotation? Both typing.Sequence[bool] | typing.Sequence[int] | typing.Sequence[float] and typing.Sequence[bool | int | float] will be an error while executing ruff formatter. What is a proper way to these annotation? Also, collections.abc.MutableSequence[bool] | collections.abc.MutableSequence[int] | collections.abc.MutableSequence[float] and collections.abc.MutableSequence[bool | int | float] will be an error.

Road-hog123 commented 1 month ago

collections.abc.MutableSequence[bool] | collections.abc.MutableSequence[int] | collections.abc.MutableSequence[float] and collections.abc.MutableSequence[bool | int | float] will be an error.

What is the error? I don't see why either of these would be incorrect type annotations.

On further consideration the former is more correct for these methods as the sequence should be homogenous.

nutti commented 1 month ago

What is the error? I don't see why either of these would be incorrect type annotations.

This is my mistake. I have already solved this.

The issue is now fixed. Thanks for sharing your idea @Road-hog123 !