restricting dynamic attribute assignment for object backend using __slots__#700
restricting dynamic attribute assignment for object backend using __slots__#700Schefflera-Arboricola wants to merge 1 commit into
__slots__#700Conversation
Saransh-cpp
left a comment
Saransh-cpp
left a comment
There was a problem hiding this comment.
Thanks for working on this, @Schefflera-Arboricola!
creating separate _methods.pys for every backend
We definitely don't want this.
I don't think it's a good solution bcoz it might break some user code
Yes, that does not sound ideal either :(
I'll look into this soon-ish.
|
Awkward arrays already prohibit this kind of attribute assignment through https://github.com/scikit-hep/awkward/blob/main/src/awkward/highlevel.py#L1302-L1335, so awkward arrays of vectors does not have this issue that you can assign attributes dynamically, see: import awkward as ak
import vector
vector.register_awkward()
arr = ak.Array(
[
[{"x": 1.1, "y": 2.2}, {"x": 3.3, "y": 4.4}],
[],
[{"x": 5.5, "y": 6.6}],
],
with_name="Vector2D",
behavior=vector.backends.awkward.behavior,
)
arr.z = 1
>>> ... AttributeError: only private attributes (started with an underscore) can be set on arraysVector objects and NumPy arrays of vectors however allow this. For NumPy arrays of vectors I think you'd have to overwrite one of the dtype fields to mess up the vector values (I forgot if that can be done through setattr). Vector objects seem to be mutable and you can "mess up" the vector, e.g.: obj = vector.obj(x=1.1, y=2.2)
print(obj.rho)
>>> np.float64(2.459674775249769)
obj.x = 2.0
print(obj.rho)
>>> np.float64(2.973213749463701)Which I am not sure if we want to support this mutability? @Saransh-cpp is there any potential Numba reason why we would want to allow this? If not, I think the safest way would be to disallow setattr and make vector objects immutable by adjusting the VectorObject, e.g. we could add a custom |
|
Making them immutable sounds good, @pfackeldey. See #158 and #457 for more discussions on this. |
3 participants
Description
Kindly take a look at CONTRIBUTING.md.
Please describe the purpose of this pull request. Reference and link to any relevant issues or pull requests.
I was experimenting with the Vector library and saw that we can assign
zto a VectorObject2D object and it neither throws an error nor does it automatically change it to aVectorObject3D:I saw there were some conversations about adding
__slots__here.. so i started working on this.The
VectorObject2/3/4Dclasses already had__slots__correctly defined but they were not being used because their parent classes had no__slots__defined so the default__dict__was still getting created (refer https://docs.python.org/3/reference/datamodel.html#slots )-- so it was allowing adding any attribute to a VectorObject2D object, likevec.blah_blah=78.In this PR I added
__slots__ = ()to parent classes and now the above code raises an error:But, because the classes in
_methods.pyare also being used by the Awkward arrays backend-- so there were some test failures (test logs under this PR)error summary
Right now, I'm not very familiar with how the awkward arrays are integrated within Vector. I would appreciate any guidance on what's the best way to make this work without bothering the Awkward arrays backend (@henryiii @jpivarski ). Pls LMK if that's even possible without creating separate
_methods.pys for every backend, if not then feel free to close this PR.Thank you!
PS: one work-around I tried was to put
ak.Arrayfirst in the class definition for classesMomentumArray2/3/4Di.e.class MomentumArray4D(ak.Array, MomentumAwkward4D):instead ofclass MomentumArray4D(MomentumAwkward4D, ak.Array):. Although it made some tests pass, I don't think it's a good solution bcoz it might break some user code, as it would giveak.Array's methods priority overMomentumAwkward2/3/4D's methods when there will be any conflicts; and thetest_subclass_fieldswas still failing.error logs
Checklist
$ pre-commit run --all-filesor$ nox -s lint)$ pytestor$ nox -s tests)$ cd docs; make clean; make htmlor$ nox -s docs)$ pytest --doctest-plus src/vector/or$ nox -s doctests)Before Merging