Add format() method for transformations and measurements#99
Conversation
tmager
force-pushed
the
tmager/format-components
branch
2 times, most recently
from
60b7653 to
6243739
Compare
Maegereg
left a comment
Maegereg
left a comment
There was a problem hiding this comment.
This will be SUCH an improvement!
| parts.extend( | ||
| f"{name}={value}" | ||
| for name, value in default_format_attrs(self, self._FORMAT_EXCLUDED_ATTRS) | ||
| ) |
There was a problem hiding this comment.
Part of me wonders whether parentheses around the attributes would be more readable? But I'm guessing you thought about that.
There was a problem hiding this comment.
Hmm, I actually hadn't considered that specific way of formatting it; there definitely are some cases where the current formatting isn't ideal, e.g. column names containing spaces. We could also just quote all string-valued attributes? That might be clearer, especially if there are any cases where the attribute value is a tuple or something -- I'll expand format_value a bit and see if I can come up with something better.
There was a problem hiding this comment.
I've pushed an alternative version that wraps quotes around all strings (it doesn't deal with quoting -- considering that this is just for human readability, I think that's fine, if you have quotes in your column names you can figure it out); it also recurses into list- and tuple-valued attributes, so that e.g. the elements of keys on Subset and key on GetValue are formatted the same way.
Original:
. AugmentDictTransformation
. Identity
| AugmentDictTransformation
. GetValue key=TableCollection(a)
| LimitRowsPerGroupValue key=NamedTable(part1) new_key=TemporaryTable(140371022854176)
LimitRowsPerGroup grouping_column=id threshold=2
| CreateDictFromValue key=
| Subset keys=['']
| AugmentDictTransformation
. GetValue key=
| Identity
| CreateDictFromValue key=TableCollection(a)
| Subset keys=[TableCollection(name='a')]
| GetValue key=TableCollection(a)
| GetValue key=TemporaryTable(140371022854176)
| LimitRowsPerGroup grouping_column=id threshold=2
| CreateDictFromValue key=TemporaryTable(140371022851056)
| GetValue key=TemporaryTable(140371022851056)
| NonInteractivePostProcess f=<function BaseMeasurementVisitor._build_adaptive_groupby_agg_and_noise_info.<locals>.perform_groupby_agg>
DecorateQueryable preprocess_query=<function create_adaptive_composition.<locals>.preprocess_query> postprocess_answer=<function create_adaptive_composition.<locals>.postprocess_answer>
SequentialComposition d_in=2*sqrt(2) privacy_budget=oo
New:
. AugmentDictTransformation
. Identity
| AugmentDictTransformation
. GetValue key=TableCollection(a)
| LimitRowsPerGroupValue key=NamedTable(part1) new_key=TemporaryTable(139997773750640)
LimitRowsPerGroup grouping_column='id' threshold=2
| CreateDictFromValue key=''
| Subset keys=['']
| AugmentDictTransformation
. GetValue key=''
| Identity
| CreateDictFromValue key=TableCollection(a)
| Subset keys=[TableCollection(a)]
| GetValue key=TableCollection(a)
| GetValue key=TemporaryTable(139997773750640)
| LimitRowsPerGroup grouping_column='id' threshold=2
| CreateDictFromValue key=TemporaryTable(139997773744496)
| GetValue key=TemporaryTable(139997773744496)
| NonInteractivePostProcess f=<function BaseMeasurementVisitor._build_adaptive_groupby_agg_and_noise_info.<locals>.perform_groupby_agg>
DecorateQueryable preprocess_query=<function create_adaptive_composition.<locals>.preprocess_query> postprocess_answer=<function create_adaptive_composition.<locals>.postprocess_answer>
SequentialComposition d_in=2*sqrt(2) privacy_budget=oo
I don't think we should spend too much effort on handling every possible case in this MR, though -- it should be pretty painless to tweak the format later.
There was a problem hiding this comment.
String quoting seems great! I still find it a little hard to distinguish the component name from the attributes, and think parentheses would help, e.g.:
LimitRowsPerGroupValue(key=NamedTable(part1), new_key=TemporaryTable(139997773750640), LimitRowsPerGroup grouping_column='id', threshold=2)
But I agree that this is a relatively minor thing.
tmager
force-pushed
the
tmager/format-components
branch
2 times, most recently
from
26a7a85 to
6838cfd
Compare
Adds a format() method for converting transformations and measurements into human-readable strings. Most components use a default implementation based on inspecting each class to get its public properties, but some (most notably any component with multiple children) use custom formatting logic. Also adds a `tmlt.core.utils.format` module containing shared helpers used during formatting. #31
tmager
force-pushed
the
tmager/format-components
branch
from
6838cfd to
bd2e2e1
Compare
| parts.extend( | ||
| f"{name}={value}" | ||
| for name, value in default_format_attrs(self, self._FORMAT_EXCLUDED_ATTRS) | ||
| ) |
There was a problem hiding this comment.
String quoting seems great! I still find it a little hard to distinguish the component name from the attributes, and think parentheses would help, e.g.:
LimitRowsPerGroupValue(key=NamedTable(part1), new_key=TemporaryTable(139997773750640), LimitRowsPerGroup grouping_column='id', threshold=2)
But I agree that this is a relatively minor thing.
3 participants
Adds a
format()method for converting transformations and measurements into human-readable strings. Most components use a default implementation based on inspecting each class to get its public properties, but some (most notably any component with multiple children) use custom formatting logic. Also adds atmlt.core.utils.formatmodule containing shared helpers used during formatting.An example output pulled from one of the Analytics tests:
#31