#53: Added REST API calls for experiments/search (Layer 1)#54
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| """ | ||
|
|
||
| def __init__(self, base_uri: str, params: JsonObject): | ||
| super().__init__( |
There was a problem hiding this comment.
Recommendation don't use inheritance, use composition, usually better to unit test
There was a problem hiding this comment.
See latest push for an improved version.
| yield from data | ||
| return | ||
| yield from ( | ||
| el | {"tag_key": tag["key"], "tag_value": tag["value"]} |
There was a problem hiding this comment.
I think, it is not a good idea to hardcode this behavior. I think, it would be better if you can give a column a post postprocessor. The only thing, this function should do is multiply out processed columns that return multiple rows
There was a problem hiding this comment.
Great idea - this can be nice and generic!
I only think this will require some additional logic, changes, and maybe tests.
Could we move this to a follow-up PR?
There was a problem hiding this comment.
There was a problem hiding this comment.
I merged the changes related to #55 into the current PR.
| self.has_tags = has_tags | ||
| self.columns = columns + self.TAG_COLUMNS if has_tags else columns | ||
|
|
||
| def _expand(self, data: Iterable[JsonObject]) -> Iterable[JsonObject]: |
There was a problem hiding this comment.
To be honest, I would declare what to expend in the column and have a general approach for unnesting.
There was a problem hiding this comment.
This seems to constantly itch you 🙂
There is another ticket: #55 and an upcoming PR.
Should I include it into the current PR?
There was a problem hiding this comment.
I merged the changes related to #55 into the current PR.
…(Layer_1)' into feature/55-generic
…(Layer_1)' into feature/55-generic
| def __init__(self, source: str, columns: list[Column]): | ||
| self.source = source | ||
| self.columns = columns |
There was a problem hiding this comment.
names of source and columns don't really help with the understanding.
source is the name of the nested column to expand
columns is the list of sub columns to include into the expansion
There was a problem hiding this comment.
I will rename in the next push
source->input_keycolumns->output_columns
| return ( | ||
| d | additional_columns(tag) | ||
| for d in data | ||
| for tag in d.get(self.source, self._default) |
There was a problem hiding this comment.
the name tag is not really helping; maybe element instead
There was a problem hiding this comment.
Ah - thanks.
This comes from generalizing a specific solution. 🙂
| width: int, | ||
| sql_name: str = "", |
| {"key": "K2", "value": "V2"}, | ||
| ] | ||
| input: list[JsonObject] = [{"c1": 1, "tags": tags}, {"c1": 2}] | ||
| assert list(testee.process(input)) == [ |
There was a problem hiding this comment.
assign the list(testee.process(input)) to a variable; otherwise, the diffs get complicated
|
ckunki
deleted the
feature/53-Added_REST_API_calls_for_experiments_search_(Layer_1)
branch
3 participants
Closes #53
Closes #55