Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## devel #441 +/- ##
==========================================
- Coverage 61.23% 61.19% -0.04%
==========================================
Files 66 66
Lines 7359 7363 +4
==========================================
Hits 4506 4506
- Misses 2853 2857 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
mtitov
left a comment
mtitov
left a comment
There was a problem hiding this comment.
@andre-merzky added several comments
| self.__dict__['_data'].update(self._defaults) | ||
|
|
||
| self.update(from_dict) | ||
| if from_dict: |
There was a problem hiding this comment.
this is duplication, we do check inside update if provided value is non empty
There was a problem hiding this comment.
Yes, but safes a function call. I am not trying to beautify, but try to optimize :-) Same for the other ones.
|
|
||
| def __setitem__(self, k, v): | ||
| self._data[k] = self._verify_setter(k, v) | ||
| if self._check : |
There was a problem hiding this comment.
duplication, we do this check inside _verify_setter
| return object.__setattr__(self, k, v) | ||
|
|
||
| self._data[k] = self._verify_setter(k, v) | ||
| if self._check : |
There was a problem hiding this comment.
duplication (it is checked inside of _verify_setter)
| else: | ||
| self.update(self._defaults) | ||
| self.__dict__['_data'] = dict() | ||
| self.__dict__['_data'].update(self._defaults) |
There was a problem hiding this comment.
I'm hesitant to call update on a dict level and not on TypedDict level (for both deep and non deep).
class NewTD(TypedDict):
_schema = {'a': TypedDict}
_defaults = {'a': {'level1': {'level2': 123}}}
ntd = NewTD({'a': TypedDict({'level1': {'level2B': 456}})})
print(ntd)# default behavior:
# {'a': TypedDict: {'level1': TypedDict: {'level2': 123, 'level2B': 456}}}
# new behavior:
# {'a': {'level1': TypedDict: {'level2B': 456}}}
Our original (default) behavior is correct, so I would keep self.update(...)
but that's right to keep self.__dict__['_data'] = dict() here in the __init__
There was a problem hiding this comment.
Ack, this needs work still.
|
@mtitov - I should have labeled that |
|
TODO AM: separate yappi and typed dict portions. |
|
Due to the flux merge, this PR reduces to shielding the |
2 participants
Simplify performance profiling. This now profiles a part of code: