micropython icon indicating copy to clipboard operation
micropython copied to clipboard

objproperty.c: Add support for fget/fset/fdel.

Open iabdalkader opened this issue 3 years ago • 4 comments

This patch allows a sub-class to override a property setter and call the base class's property setter. Without this, the workaround would be to access what should be a private attribute of the base class (f._bar), which is not a good practice. Tested with the following:

paste mode; Ctrl-C to cancel, Ctrl-D to finish
=== class Foo():
===     def __init__(self):
===         self._bar = 0
=== 
===     @property
===     def bar(self):
===         return self._bar
=== 
===     @bar.setter
===     def bar(self, value):
===         self._bar = value
=== 
=== class FooBar(Foo):
===     @Foo.bar.setter
===     def bar(self, value):
===         Foo.bar.fset(self, value)
=== 
=== f = FooBar()
=== f.bar = 5
=== print(f.bar)
5
>>> 

iabdalkader avatar Jul 07 '22 15:07 iabdalkader

Codecov Report

Merging #8881 (8c7ca47) into master (5c31a6c) will decrease coverage by 0.00%. The diff coverage is 70.00%.

@@            Coverage Diff             @@
##           master    #8881      +/-   ##
==========================================
- Coverage   98.31%   98.30%   -0.01%     
==========================================
  Files         157      157              
  Lines       20349    20359      +10     
==========================================
+ Hits        20006    20014       +8     
- Misses        343      345       +2     
Impacted Files Coverage Δ
py/objproperty.c 92.30% <70.00%> (-7.70%) :arrow_down:
py/runtime.c 99.85% <0.00%> (+0.14%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 5c31a6c...8c7ca47. Read the comment docs.

codecov-commenter avatar Jul 07 '22 15:07 codecov-commenter

This will need tests, but I guess we can decide whether it's worth merging first. FWIW +76 bytes on PYBV11.

jimmo avatar Jul 08 '22 02:07 jimmo

Seems worth having, but probably the use-cases are so scarce that it needs a PP guard?

stinos avatar Jul 08 '22 06:07 stinos

I guess we can decide whether it's worth merging first.

If it's approved I'll add the required tests.

iabdalkader avatar Jul 08 '22 09:07 iabdalkader