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

Cli unit conversion for adding data #341

Merged
merged 6 commits into from Jan 31, 2022
Merged

Conversation

Flix6x
Copy link
Contributor

@Flix6x Flix6x commented Jan 29, 2022

This PR adds a CLI option to pass a data unit when reading in time series data from CSV, so data can automatically be converted to the sensor unit. Also allows to swap signs.

Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
@Flix6x Flix6x added the CLI label Jan 29, 2022
@Flix6x Flix6x self-assigned this Jan 29, 2022
Signed-off-by: F.N. Claessen <felix@seita.nl>
@Flix6x
Copy link
Contributor Author

Flix6x commented Jan 29, 2022

@nhoening I'm not sure whether you intend to have CLI changelog entries for these kind of changes?

@Flix6x Flix6x requested a review from nhoening January 29, 2022 15:19
Copy link
Contributor

@nhoening nhoening left a comment

Choose a reason for hiding this comment

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

Is this new function covered by tests, probably sensor schema or API tests?

Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
…it conversion

Signed-off-by: F.N. Claessen <felix@seita.nl>
@Flix6x
Copy link
Contributor Author

Flix6x commented Jan 31, 2022

Yes, by test_post_sensor_data. But I now also added unit tests (pun intended), which then led me to fix temperature conversion.

@Flix6x Flix6x requested a review from nhoening January 31, 2022 12:19
@Flix6x Flix6x merged commit 9ad05aa into main Jan 31, 2022
@Flix6x Flix6x deleted the cli-unit-conversion-for-adding-data branch January 31, 2022 15:34
@Flix6x Flix6x added this to the 0.9.0 milestone Jan 31, 2022
@Flix6x Flix6x added the Data label Jan 31, 2022
@Flix6x Flix6x mentioned this pull request Jan 31, 2022
Flix6x added a commit that referenced this pull request Feb 1, 2022
Add a test for switching signs and fix the regression (from #341) that broke that conversion. Also add a test for another unit that includes a scaling factor (l/100km, used for car fuel consumption), and comment out two test cases that show the current limitations of our unit conversion function.


* Use FlexMeasures unit registry

Signed-off-by: F.N. Claessen <felix@seita.nl>

* Fix and test conversion from -W to W

Signed-off-by: F.N. Claessen <felix@seita.nl>

* Refactor

Signed-off-by: F.N. Claessen <felix@seita.nl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants