Skip to content
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

const/immutable types #178

Open
esoma opened this issue Mar 3, 2022 · 3 comments
Open

const/immutable types #178

esoma opened this issue Mar 3, 2022 · 3 comments
Assignees

Comments

@esoma
Copy link
Contributor

esoma commented Mar 3, 2022

I'd really like to see immutable version of existing vec/mat/quat/array types. Personally, I have 2 use cases for such types:

Immutable objects that contain glm objects

I have objects which are immutable which have pyglm object as attributes. Since pyglm objects are mutable, whenever a user wants to access one of these attributes it requires making a copy of the pyglm object so that the parent object remains publicly immutable. For example:

class MyImmutableObject:
    def __init__(self, point: vec3):
        self._point = point

    @property
    def point(self) -> vec3:
        return vec3(self._point)

This isn't the worst thing ever, but I also have cases where I'm doing this with rather large arrays, which is pretty wasteful.

Hashing

I've encountered situations where I'd like to use pyglm objects as dictionary keys and other situations where I've wanted to eliminate duplicates. Since pyglm objects are mutable they can't be hashed, so I've had to resort to converting them back and forth from tuples in many cases.

unique_points = array(vec3(p) for p in {tuple(p) for p in non_unique_points})

Immutable versions of pyglm object could be hashable and eliminate these needless conversions.

Implementation

There is some precedent for this already with array having readonly. As far as I know it isn't really possible to construct a read only array through the python API though. If all objects had a readonly attribute that could be set on creation with a keyword only argument that may work?

point = vec3(0, readonly=True)

Personally, I'd rather see a whole extra type to avoid the overhead from readonly checks. Something like:

point = const_vec3(0)

I think math operations would continue to return the normal objects (so adding two readonly vec3 would result in a regular vec3) to keep things simple.


Thoughts? I'm happy to work on this if it sounds reasonable.

@Zuzu-Typ
Copy link
Owner

Zuzu-Typ commented Mar 3, 2022

Hey there (:

I have objects which are immutable which have pyglm object as attributes. Since pyglm objects are mutable, whenever a user wants to access one of these attributes it requires making a copy of the pyglm object so that the parent object remains publicly immutable.

I see your point. I could see this being a useful feature.
It would not be possible to make it truly immutable though, because it would always be possible to get the glm.value_ptr() and modify its contents or forward it to a C / C++ library function that modifies it. Though the python object could be made immutable.

I've encountered situations where I'd like to use pyglm objects as dictionary keys and other situations where I've wanted to eliminate duplicates. Since pyglm objects are mutable they can't be hashed, so I've had to resort to converting them back and forth from tuples in many cases.

Generally, PyGLM objects are hashable. However, it is dangerous to use mutable objects as dictionary keys:

import glm

v = glm.vec3(1, 2, 3)
print( hash( v ) ) # 1443159451135942609
# no problem



k = glm.ivec2(8, 3)
d = { glm.ivec2( k ) : v }
k.y = 9

k_from_d = tuple( d.keys() )[0]

print( k_from_d == k ) # False
print( k_from_d in d ) # True
# (no) problem

k_from_d.y = 9

print( k_from_d == k ) # True
print( k_from_d in d ) # False
# problem



k = glm.ivec2(8, 3)
d = { k : v }
k.y = 9

k_from_d = tuple( d.keys() )[0]

print( k_from_d == k ) # True
print( k_from_d in d ) # False
# problem

As for the implementation...
In theory, the best way of implementing a readonly object is by not having any setter methods. This requires an extra type though. Much like the const_vec3 type you proposed.
This has the benefit of not requiring any additional readonly flags or checks.
There is one downside though, I wouldn't want to change the existing implementation for the operators, which means applying any operation to a const_vec would always result in a normal vec. That shouldn't be a big problem though.

I'm more worried about my current type-checker implementation. Due to the way it works, having a two parallel types would be problematic in some cases.
I'm currently working on an improved type-checker that should not have these problems.

Having a readonly flag is also a possibility, though it would have a little performance overhead and the same problem with operators (i.e. applying any operation to a readonly vec would yield a non-readonly vec).

I haven't quite made up my mind yet, which option is the best.

@Zuzu-Typ Zuzu-Typ self-assigned this Mar 3, 2022
@esoma
Copy link
Contributor Author

esoma commented Mar 3, 2022

It would not be possible to make it truly immutable though, because it would always be possible to get the glm.value_ptr() and modify its contents or forward it to a C / C++ library function that modifies it.

Agreed. My goal is just a best effort at preventing mistakes in the same way that we might return a const tvec3& in C++, even that can be bypassed with casting. My "immutable" objects aren't truly immutable either in that sense, since we could just access the _point attribute directly.

glm.value_ptr() is a little different since it could still be done without any "positive action" from the user to override the intended behavior. Maybe a glm.const_value_ptr() function could be added, same as the current glm.value_ptr(). glm.value_ptr() would then get a check to make sure the object isn't readonly. The "protection" with that would be pretty limited, only at the call site for value_ptr/const_value_ptr, so I'm not sure that extra complication is worth it. Plus is it already the case that you can get a glm.value_ptr() for an array that is readonly? I think if you're working with ctypes then "const-ness" is something the Python community has already accepted as just needing to be externally aware of since ctypes itself doesn't support it. Not much can be done about it, I think.

Generally, PyGLM objects are hashable.

Interesting. I guess I just assumed they weren't. Good to know.

@szabolcsdombi
Copy link
Contributor

I use pyglm in my toy examples a lot. these are examples for newcommers, beginners, ...
For some reason I keep forgetting that pyglm objects are mutable and tend to reference object position, rotation when I actually want a snapshot of this data at a given time.
To be less generic here is really weird use case I encountered:

class Player:
    def update(self):
        self.position += self.forward * self.speed


def spawn_footprint(player):
    world.add(Footprint(player.position))

the code above spawns flying footprints following the player.

Adding an offset to the position or removing it later changes this behavior.
In my opinion this is very error prone, this also happens with lists and not with tuples, I know.

Using some const_vec3 type or readonly=... parameter just violates simplicity.

I understand the efficiency of inplace operations, the cost of allocating new objects, but I think using it for pyglm causes more harm than help.

My point is that, even I can avoid these pitfalls, others may reuse/change the code and find weird hard-to-explain behavior.

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

No branches or pull requests

3 participants