Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add pythonization for freezing class attribute and preventing creating new attrubes by mistake #663

Merged
merged 3 commits into from
Sep 6, 2024

Conversation

m-fila
Copy link
Contributor

@m-fila m-fila commented Sep 6, 2024

BEGINRELEASENOTES

  • Added pythonization for "freezing" class disallowing setting non-existent attributes

ENDRELEASENOTES

Pythonization proposed in key4hep/EDM4hep#357. Adding new attributes to existing objects is a feature in python. Unfortunately in the context of podio this is a very error prone behaviour that already lead to some bugs in the past.

For instance a typo in attribute name will silently create new attribute. With this pythonization an exception will be thrown
The error message is consistent with __slots__ which is conventionally used in standard python to construct classes with frozen attributes

@hegner hegner self-requested a review September 6, 2024 10:03
@hegner
Copy link
Collaborator

hegner commented Sep 6, 2024

@m-fila - always on the wish list and you did it - great! :-)

@hegner hegner merged commit 27e92d5 into AIDASoft:master Sep 6, 2024
18 checks passed
Copy link
Collaborator

@tmadlener tmadlener left a comment

Choose a reason for hiding this comment

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

Just for my understanding. This is still opt-in for generated EDMs, right? And I only don't see the "opting in" in the diff of this PR, because for podio tests we already have opted in with the other pythonizations?

Is it possible to selectively opt in to the pythonizations?

@m-fila
Copy link
Contributor Author

m-fila commented Sep 6, 2024

Yes, it's opt-in. That test opted for all the pythonizations in podio for namespace/module nsp

from pythonizations import load_pythonizations # pylint: disable=import-error
# load all available pythonizations to the classes in a namespace
# loading pythonizations changes the state of cppyy backend shared by all the tests in a process
load_pythonizations("nsp")

Loading specific pythonization is also supported

from  podio.pythonizations import freeze_class

freeze_class.FreezeClassPythonizer.register("edm4hep")

The docs are already there https://key4hep.web.cern.ch/podio/python.html#pythonizations 😉

@m-fila m-fila deleted the prevent_add_attr branch September 6, 2024 11:11
@tmadlener
Copy link
Collaborator

The docs are already there https://key4hep.web.cern.ch/podio/python.html#pythonizations 😉

😂😂 you would think people look there first before asking stupid questions ;)

Thanks for the clarifications.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants