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

642 add report cli command #656

Closed

Conversation

victorgarcia98
Copy link
Contributor

@victorgarcia98 victorgarcia98 commented May 1, 2023

This PR introduces the command flexmeasures add report which allows to create reports (custom calculations defined in classes of type Reporter) and save them to the database or export them to Excel or CSV. It also includes some flags to define the time scope of the report in some predefined ways: --last-hour, --last-day, ...

Closes #642.

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
@victorgarcia98 victorgarcia98 marked this pull request as ready for review May 1, 2023 14:44
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
@victorgarcia98 victorgarcia98 added enhancement New feature or request Reporting labels May 1, 2023
@victorgarcia98 victorgarcia98 self-assigned this May 1, 2023
@Flix6x
Copy link
Contributor

Flix6x commented May 1, 2023

>1500 lines of code to review is a bit much. :) I think of you update the PR to merge to the reporter branch instead of to main, we should get a nicer separation of the two reviews. When we merge the reporter branch (which will likely happen before this one) the base branch will be automatically updated.

One more thing, have you considered adding review instructions? Like, "here's a cool CLI command that this PR makes possible". Otherwise, I'd start as usual, looking at the tests.

@victorgarcia98 victorgarcia98 changed the base branch from main to 626-add-reporter-class May 2, 2023 06:14
@victorgarcia98 victorgarcia98 changed the base branch from 626-add-reporter-class to main May 2, 2023 06:32
@victorgarcia98 victorgarcia98 changed the base branch from main to 626-add-reporter-class May 2, 2023 06:32
@victorgarcia98
Copy link
Contributor Author

>1500 lines of code to review is a bit much. :) I think of you update the PR to merge to the reporter branch instead of to main, we should get a nicer separation of the two reviews. When we merge the reporter branch (which will likely happen before this one) the base branch will be automatically updated.

One more thing, have you considered adding review instructions? Like, "here's a cool CLI command that this PR makes possible". Otherwise, I'd start as usual, looking at the tests.

Yes, It wasn't looking good... Thanks for the advice to clean it up.

@nhoening
Copy link
Contributor

nhoening commented May 2, 2023

@victorgarcia98 why did you close this PR?

@victorgarcia98
Copy link
Contributor Author

@victorgarcia98 why did you close this PR?

I messed it up adding commits from main and the other branch. I've just opened PR #659 with the changes that I'm proposing.

@Flix6x Flix6x deleted the 642-add-report-cli-command branch May 15, 2023 13:13
@Flix6x Flix6x restored the 642-add-report-cli-command branch May 15, 2023 13:14
@Flix6x Flix6x deleted the 642-add-report-cli-command branch May 15, 2023 13:14
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.

Add CLI command flexmeasures add report to compute reports using a Reporter
3 participants