Skip to content
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

179: add-model-to-sites-table #181

Closed

Conversation

hanaawad24
Copy link

Pull Request

Description

This PR addresses the issue #179 to handle the ability to assign a model to one or more site.

@hanaawad24 hanaawad24 force-pushed the add-model-to-sites-table branch 5 times, most recently from 52f7d4c to 062c8c7 Compare November 27, 2024 23:13
@hanaawad24 hanaawad24 force-pushed the add-model-to-sites-table branch 2 times, most recently from 223948b to d42dca6 Compare November 27, 2024 23:17
@peterdudfield
Copy link
Contributor

Thanks so much @hanaawad24 for doing this.

I've added this in already - https://github.com/openclimatefix/pv-site-datamodel/blob/main/pvsite_datamodel/write/user_and_site.py#L381

I wondered if you could improve the current function, and include those extra tests you have added?

nullable=True,
comment="The ML Model which should be used for this site",
)
model_uuid = sa.Column(UUID(as_uuid=True), sa.ForeignKey("ml_model.model_uuid"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you remove this change, the nullable=True, and the comment is useful

@peterdudfield
Copy link
Contributor

I'll close this for the moment, but feel free to add those test improvements on this PR, or a new one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants