Small changes in dynamic properties

[leoguignard]

While giving the code review, @jules-vanaret pointed one mistake that I corrected.
Moreover, I also saw few redundancies that are now corrected also.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.77%. Comparing base (750b7c9) to head (b363fb3).
Report is 5 commits behind head on v2.x.

Additional details and impacted files

@@           Coverage Diff           @@
##             v2.x      #73   +/-   ##
=======================================
  Coverage   96.77%   96.77%           
=======================================
  Files           2        2           
  Lines         341      341           
  Branches       17       17           
=======================================
  Hits          330      330           
  Misses          6        6           
  Partials        5        5           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[jules-vanaret]

@leoguignard, to me the changes look good, but I have a question about another part of dynamic_property (I'll write it here as I can't attach a message to these lines in the "Files changed" tab as they are not part of the review).

In the the dynamic_property class definition, isn't setattr(owner, self.protected_name, None) (from __set_name__) redundant with if getattr(instance, self.protected_name, None) is None: (from __get__) ? In the latter you already manage the case for which instance.protected_name is not defined.

[leoguignard]

I agree it's redundant indeed. I'll remove it in another PR since it's not breaking and it's actually not changing the processing time (or at most only very marginally).

[leoguignard]

@jules-vanaret, @BadPrograms, @ChrisMzz, Can I have one of you reviewing the PR so I can merge if you are ok with the changes?
Cheers :)

[ChrisMzz]

Looks good to me :)

[ChrisMzz]

Is there a reason why you don't remove should_reset and replace L85 this with the following?

if not (hasattr(self, "_has_been_reset") and self._has_been_reset):

If not I think it might look nicer.