search

pywr / pywr-next

An experimental repository exploring ideas for a major revision to Pywr using Rust as a backend.
6 stars 4 forks source link

fix: schema refs to v1 params that are converted to timeseries inputs are now updated to timeseries refs #191

Closed Batch21 closed 4 weeks ago

Batch21 commented 1 month ago

Another possible way to do this would be to extract all the Timeseries references first and pass that to the node/param conversion methods but I think that might be a messier approach as we wouldn't be able to use the TryFrom implementations for the conversions.

It might be worth updating visit_metrics_mut so that errors in this update process can be captured. What do you think?

Batch21 commented 1 month ago

This looks good. Is it worthwhile adding a test for this kind of thing?

done

jetuk commented 1 month ago

It might be worth updating visit_metrics_mut so that errors in this update process can be captured. What do you think?

I'm also not sure what you mean by this? Can you give an example?

Batch21 commented 1 month ago

It might be worth updating visit_metrics_mut so that errors in this update process can be captured. What do you think?

I'm also not sure what you mean by this? Can you give an example?

I was thinking about updating the trait method to return a Result so that any errors in converting a parameter ref to a timeseries ref can be added to the conversion errors Vec and returned to the user. Errors can occur if the TimeseriesColumn struct cannot be generated from the v1 data. I've seen a few of these because, unlike v1, v2 requires a column name attribute for CSVs with only one column of data . Currently, in this situation, the code just bails on the conversion to a timeseries ref but I think it would be more useful to show this as an error to the user.