-
Notifications
You must be signed in to change notification settings - Fork 145
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Tidy] Improve code style in response models #626
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, thanks for tackling those already - here are my initial thoughts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was more imagining something along the following lines:
from typing import Any
try:
from pydantic.v1 import BaseModel, Field, create_model
except ImportError: # pragma: no cov
from pydantic import BaseModel, Field
class FooModel(BaseModel):
foo: str
bar: int = 123
class ProxyModel:
def __new__(cls, color) -> Any:
return create_model(
'BarModel',
apple='russet',
banana=color,
__base__=FooModel,
)
Proxy = ProxyModel(color="orange")
print(Proxy.__fields__.keys())
print(Proxy.__fields__.values())
So in the above toy example, Proxy
can just act as a normal response model no?
def _get_target_df_name(self, components_plan, controllable_components): | ||
target_controllable = set(self.target_components_id) & set(controllable_components) | ||
df_names = { | ||
component_plan.df_name | ||
for component_plan in components_plan | ||
if component_plan.component_id in target_controllable | ||
} | ||
|
||
if len(df_names) > 1: | ||
logger.warning( | ||
f""" | ||
[FALLBACK] Multiple dataframes found in the target components: {df_names}. | ||
Choose one dataframe to build the filter. | ||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I do not understand this. Should the logic not be:
- model determines which components should be controlled
- we then check if those components can be controlled, and take a subset at worst
- the remaining dfs are what we get the metadata for (and the could be multiple no?)
Description
Addressed 2 TODOs in
controls.py
, as per discussion with @maxschulz-COL, to make response models more consistent in stylesPlus 2 minor cleanup in
components.py
andlayout.py
Screenshot
Notice
I acknowledge and agree that, by checking this box and clicking "Submit Pull Request":