search

openclimatefix / pv-site-datamodel

Datamodel for PV sites
MIT License
2 stars 9 forks source link

Create an edit_site function #144

Closed Bvr4 closed 4 months ago

Bvr4 commented 4 months ago

Detailed Description

Create a function to edit site metadata

Context

This function will be useful to the pv-site-api, specially for the issue 178 : edit site

Possible Implementation

here : https://github.com/openclimatefix/pv-site-datamodel/blob/main/pvsite_datamodel/write/user_and_site.py

Bvr4 commented 4 months ago

I'd like to be assigned to this issue as I am working on the pv-site-api side of this issue.

Should this function update all fields, or only those that are filled in? (ignoring those set to None)

peterdudfield commented 4 months ago

I think we should update all the fields even if None. An odd edge case is, we might want to se some fields back to None. Do you think that'll work?

Bvr4 commented 4 months ago

I hadn't thought of this edge case. It's easier to update all the fields whatever.

However, we need to make sure that the user reads the site values before updating them, so that he knows the current values of all the fields.

peterdudfield commented 4 months ago

Yea I agree, perhaps a different way to attack the problem, is a dictionary gets passed in, and this dictionary is the keys and values that are to be updated

Bvr4 commented 4 months ago

Something like a pydantic PVSiteEditMetadata model, with all fields set to Optional ?

peterdudfield commented 4 months ago

yea soudn about right. Does PVSiteEditMetadata site here on in the api?

Bvr4 commented 4 months ago

In my opinion, as PVSiteEditMetadata will be used here and in the API, it should be defined here, and imported in the API.

But looking at how the existing "create_site" part is handled, the API has a pydantic model defined, but the create_site part in pv_site_datamodel has no pydantic model as input, only parameters. Which may seem a bit odd.

Sorry, these may be details, but I don't want to mess up the code base, and stay consistent with it.

peterdudfield commented 4 months ago

yea sure, no worries. This could just be how the code base was built up.

I would be tempted to try moving PVSiteEditMetadata here, and building the function around that.