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

Aggregate reports by day #231

Merged
merged 4 commits into from
Jan 2, 2019
Merged

Aggregate reports by day #231

merged 4 commits into from
Jan 2, 2019

Conversation

brandomr
Copy link
Contributor

@brandomr brandomr commented Nov 4, 2018

This PR is in response to feature request #216. It adds an optional flag (--daily or -D) to the CLI report command. This aggregates output for each day.

This is accomplished by iteratively calling the report function for each day in the specified (or default) reporting time span.

The --daily flag is mutually exclusive to the --json flag so only one or the other option may be specified.

To test this PR, you may clone my fork and checkout my branch (report-by-day). Then you should install it in a virtual environment or conda environment and run:

watson report --daily

I am open to suggestions on better ways to accomplish this or ideas on how to improve its formatting. I recognize that my solution, though it works, is not the most elegant. So, happy to take some tips! Also, I did not see tests for cli.py so was not sure how this should be tested, if at all, since no watson or utils functions were changed.

@brandomr brandomr mentioned this pull request Nov 7, 2018
@jmaupetit jmaupetit requested a review from k4nar November 8, 2018 08:55
@jmaupetit
Copy link
Contributor

Thank you for this contribution @brandomr!

@k4nar I've assigned you to review this PR, please tell me if you don't have time to do so.

@@ -345,6 +368,10 @@ through the `--pager` option.
You can change the output format for the report from *plain text* to *JSON*
by using the `--json` option.

Reporting by day can be created by using the `--daily` or `-D` flag.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a specific reason for choosing -D instead of -d?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I chose -D since the -d flag is already used to report for the current day. See https://github.com/TailorDev/Watson/blob/master/watson/cli.py#L341-L344

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh yes I missed it :) .

Copy link
Collaborator

@k4nar k4nar left a comment

Choose a reason for hiding this comment

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

This works great, but I fear it adds too much complexity to the command :( .

I wonder if it shouldn't be a totally separated command. In the end, what you want to do is a command aggregating some watson report over a period of time. Maybe watson aggregate then? We can probably find a better name though 😄 .

For examples we could have use cases like:

$ watson aggregate --day --from 2018-01-16 --to 2018-03-16
# equivalent of calling watson report --day for every day in the given timespan
...
$ watson aggregate --year --occurences 3
# equivalent of calling watson report --year 3 times with the dates of the past years
...

Now I don't know if you would be up to implementing something like that, but given some time I might be able to work on it :) .

@jmaupetit : WDYT?

@brandomr
Copy link
Contributor Author

brandomr commented Nov 9, 2018

@k4nar I agree the watson report command is pretty complex. To be honest as a new user it took me a bit of learning to get used to it! Having watson aggregate as a separate command would be nice and I don't think a ton of work to do. That said...this does work 😉

`-f, --from DATE` | Date and time of start of tracked activity [required]
`-t, --to DATE` | Date and time of end of tracked activity [required]
`--help` | Show this message and exit.

Copy link
Contributor

Choose a reason for hiding this comment

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

🤔 looks like the documentation was not up-to-date.

@jmaupetit
Copy link
Contributor

@brandomr that's a great piece of work! 👏

After digging into it, I think making it a separated command would make it easier to maintain. I am sorry to ask you this, but is there any chance that you refactor your work consequently?

@brandomr
Copy link
Contributor Author

@jmaupetit I think that makes the most sense for the project overall. I don't think it will be too hard to do and should have that done by the end of next week!

@jmaupetit
Copy link
Contributor

@brandomr
Copy link
Contributor Author

brandomr commented Nov 21, 2018

@jmaupetit and @k4nar I refactored so that we now have a watson aggregate command. @k4nar I liked your suggestion about having the option to aggregate (for example) 3 years or 2 months, etc, however I think this will take substantially more work.

For now, this simply aggregates a report by day for the specified time frame (or the default time frame of 1 week). It supports the JSON, tags, project, and pager options as well, but does not have options for (month, day, year, all, luna).

For example you can run:

watson aggregate --from=2018-10-01 -p myfunproject -j

to return JSON output aggregated by day from 2018-10-01 for the project "myfunproject".

I think this is a good CLI command which we can build out with new features over time. Let me know what you guys think.

watson/cli.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@k4nar k4nar left a comment

Choose a reason for hiding this comment

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

This looks very great overall 👍 . That's a shame we still don't have tests for the cli though 😄 .

@brandomr
Copy link
Contributor Author

brandomr commented Dec 6, 2018

@k4nar @jmaupetit just wanted to bump this 😄!

k4nar
k4nar previously approved these changes Jan 2, 2019
Copy link
Collaborator

@k4nar k4nar left a comment

Choose a reason for hiding this comment

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

🍾

@k4nar k4nar merged commit 01026f9 into jazzband:master Jan 2, 2019
andrewmeyer added a commit to andrewmeyer/Watson that referenced this pull request Jan 2, 2019
jmaupetit added a commit that referenced this pull request Mar 25, 2019

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Added:

- New `add` command (#202)
- Add lunar start time options to the `report` and `log` commands (#215)
- Aggregate reports by day (#231)
- Fish shell completion (#239)
- Add support for first day of the week configuration in reports and logs (#240)
- Python 3.7 support (#241)
- Add `start --no-gap` and `stop --at` options (#254)

Changed:

- The `edit` command now checks data consistency (#203)
- Current state saving is now improve when using Watson as a library (#214)
- Prevent calling `get_start_time_for_period` multiple times (#219)

Fixed:

- Improved support for UTF-8 with Python 2 (#211)
- Zsh completion for tags and projects with spaces in their names (#227)
- Typos in commands output (#230, #235)
- Project URL of the project in PyPI (#260)

Removed:

- Python 3.3 support (#210).
@jmaupetit jmaupetit mentioned this pull request Mar 25, 2019
jmaupetit added a commit that referenced this pull request Mar 25, 2019
Added:

- New `add` command (#202)
- Add lunar start time options to the `report` and `log` commands (#215)
- Aggregate reports by day (#231)
- Fish shell completion (#239)
- Add support for first day of the week configuration in reports and logs (#240)
- Python 3.7 support (#241)
- Add `start --no-gap` and `stop --at` options (#254)

Changed:

- The `edit` command now checks data consistency (#203)
- Current state saving is now improve when using Watson as a library (#214)
- Prevent calling `get_start_time_for_period` multiple times (#219)

Fixed:

- Improved support for UTF-8 with Python 2 (#211)
- Zsh completion for tags and projects with spaces in their names (#227)
- Typos in commands output (#230, #235)
- Project URL of the project in PyPI (#260)

Removed:

- Python 3.3 support (#210).
jmaupetit added a commit that referenced this pull request Mar 25, 2019
Added:

- New `add` command (#202)
- Add lunar start time options to the `report` and `log` commands (#215)
- Aggregate reports by day (#231)
- Fish shell completion (#239)
- Add support for first day of the week configuration in reports and logs (#240)
- Python 3.7 support (#241)
- Add `start --no-gap` and `stop --at` options (#254)

Changed:

- The `edit` command now checks data consistency (#203)
- Current state saving is now improve when using Watson as a library (#214)
- Prevent calling `get_start_time_for_period` multiple times (#219)

Fixed:

- Improved support for UTF-8 with Python 2 (#211)
- Zsh completion for tags and projects with spaces in their names (#227)
- Typos in commands output (#230, #235)
- Project URL of the project in PyPI (#260)

Removed:

- Python 3.3 support (#210).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants