Conversation
…where i found a get_derived_from
|
sorry for the slightly chaotic commit history, it's midnight here |
|
|
||
| def _lookup_possibly_derived_attribute(self, attr): | ||
| derived_from = self.get_derived_from() | ||
|
|
There was a problem hiding this comment.
Remove superfluous whitespace.
| if(f.get_derived_from() == None): | ||
| return f | ||
| else: | ||
| return f.get_derived_from() |
There was a problem hiding this comment.
As is, this will raise a different exception in the case that would have previously raised a KeyError. I suggest the following as an alternative that maintains the existing behavior. This also avoid multiple calls to get_derived_from() which might recurse on that path (not expecting that performance would be worth concerning ourself with too much but it exists):
for field in self.parent.fields:
if field.name == self.derived_from:
next_level_derive = field.get_derived_from()
return next_level_derive if next_level_derive else fieldThere was a problem hiding this comment.
hello paul, as i just ran it on the whole corpus of data as-is and it doesn't raise any exception for me, on which file do you have that ?
|
Logic of change looks good; if you agree please incorporate the proposed simplification and then squash your commits down to clean up the history. Two commits are fine with the first being the failing test addition followed by the changes to support derivative recursion. |
|
just saw your answer, will take care of that tomorrow |
|
How's the progress of this PR? I ran in to the problem of register clusters not being propagated through |
|
Closing in favor of #172 |
Paul
fixed for peripherals (for which i had a test case),
while i was in : also fixed for registers, register clusters and register cluster arrays
BUT there is no test case for this, do you have something ?
you weren't accounting for nesting so i recursed a bit