[PNE-6367] Add support for feature effects.#982
Conversation
| The query used to define the materials underpinning this table | ||
| generation_algorithm: TableFromGemdQueryAlgorithm | ||
| Which algorithm was used to generate the config based on the GemdQuery results | ||
|
|
There was a problem hiding this comment.
My local flake8 was complaining about this file, despite me not touching it. 🤷♂️
1d2b4cb to
8a16228
Compare
The payload comes in as a condensed format, so it's expanded in order to constructed nested lists of objects for clarity and ease of use. Additionally, the hierarchy of data is flipped to more closely match how it will be used by our customers. To that end, 'as_dict' is provided to ease importing it into a pandas DataFrame for whatever processing and analysis the customer desires.
8a16228 to
623a307
Compare
kroenlein
left a comment
There was a problem hiding this comment.
Mostly good. I have a concern about a sig change.
| @property | ||
| def as_dict(self) -> Dict[str, Dict[str, Dict[UUID, float]]]: | ||
| """Presents the feature effects as a dictionary by output.""" | ||
| return {output.output: output.feature_dict for output in self.outputs} |
There was a problem hiding this comment.
resource.py supports an as_dict method that is not a property. Does it need to be a property? I'm concerned about changing the signature on a derived class.
There was a problem hiding this comment.
Resource doesn't define as_dict, just GEMDResource.
There was a problem hiding this comment.
Yes, you're right. Not a sig collision.
Choice of property or method doesn't seem to have a consistent flavor in the library. This feels more like a method to me than a property, but acceptable as is if you feel strongly to the contrary.
The payload comes in as a condensed format, so it's expanded in order to contructed nested lists of objects for clarity and ease of use.
Additionally, the hierarchy of data is flipped to more closely match how it will be used by our customers. To that end, 'as_dict' is provided to ease importing it into a pandas DataFrame for whatever processing and analysis the customer desires.
PR Type:
Adherence to team decisions