On 9/23/20 12:35 PM, Eduardo Habkost wrote:
I believe these two lines above should be removed, as suggested
in patch 30, but let's ignore that for now.
Yup, headed there.
- if features:
- obj['features'] = [(f.name, Extra(None, f.ifcond)) for f in features]
I can't say I understand completely why moving these two lines
outside _make_tree() is useful, but if it makes the cleanup work
you did easier, I trust this is the right thing to do. The
changes look correct.
The basic premise is:
Why pass information you want to add to obj['features'] to a function to
make that assignment, when you could just perform that assignment yourself?
Otherwise, _make_tree, which accepts any arbitrary object (not just
dicts!) has to interrogate its arguments to make sure you gave it a dict
when you give it a features argument.
Type-wise, it's cleaner to perform this transformation when we KNOW we
have an object than it is to defer to a more abstracted function and
assert/downcast back to more specific types.
