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

Rethinking get_custom_effort and get_effort #172

Closed
damianooldoni opened this issue Sep 14, 2022 · 6 comments
Closed

Rethinking get_custom_effort and get_effort #172

damianooldoni opened this issue Sep 14, 2022 · 6 comments
Milestone

Comments

@damianooldoni
Copy link
Member

While reviewing get_*() functions vs features of map_dep() (see #171) I found the function name get_custom_effort() very misleading.

@jimcasaer, @LynnINBO and I added get_custom_effort() to allow the user to select a start date, an end date and also a group_by interval, e.g. "days". See #70.
Notice that such effort is calculated over all deployments (filtering by predicates is of course possible)!

But why not adding start, end and group_by to get_effort() as well and renaming get_custom_effort() as get_total_effort() or get_global_effort()?

Of course users can continue to use get_custom_effort, but its usage will be deprecated.

@jimcasaer, @LynnINBO, @peterdesmet: what do you think about it?

@damianooldoni damianooldoni added this to the version 1 milestone Sep 14, 2022
@damianooldoni
Copy link
Member Author

damianooldoni commented Sep 15, 2022

Decided with @jimcasaer:

  • yes, start, end and group_by fit perfectly get_effort(). The default values (start = NULL, end = NULL, group_by = NULL) will return the effort as it is returned now and will be by far the most typical usage.
  • agree to rename get_custom_effort. However, get_global_effort() doesn't soudn good for @jimcasaer as it reminds a calculation over the globe, the entire Earth, not anymore project wise.
  • @jimcasaer's alternatives: get_project_effort() (I give 👍 ) or get_overall_effort() (I give 👎 ).

@peterdesmet: which name is the best, do you think? Other alternatives are welcome.

@peterdesmet
Copy link
Member

What is the difference in functionality of get_effort(start, end, group_by) vs get_project_effort()?

@damianooldoni
Copy link
Member Author

damianooldoni commented Sep 15, 2022

get_effort is at deployment level, the actual get_custom_effort() (to be renamed maybe as get_project_effort()) is over all deployments. Default examples:

library(camtraptor)

get_effort(mica)
#> # A tibble: 4 × 4
#>   deploymentID                         effort unit  effort_duration       
#>   <chr>                                 <dbl> <chr> <Duration>            
#> 1 29b7d356-4bb4-4ec4-b792-2af5cc32efa8   239. hour  859859s (~1.42 weeks) 
#> 2 577b543a-2cf1-4b23-b6d2-cda7e2eac372   219. hour  786802s (~1.3 weeks)  
#> 3 62c200a9-0e03-4495-bcd8-032944f6f5a1   529. hour  1903602s (~3.15 weeks)
#> 4 7ca633fa-64f8-4cfc-a628-6b0c419056d7   335. hour  1204929s (~1.99 weeks)

get_custom_effort(mica)
#> # A tibble: 1 × 3
#>   begin      effort unit 
#>   <date>      <dbl> <chr>
#> 1 2019-10-09  1321. hour

get_custom_effort(mica, group_by = "day")
#> # A tibble: 558 × 3
#>    begin      effort unit 
#>    <date>      <dbl> <chr>
#>  1 2019-10-09   12.7 hour 
#>  2 2019-10-10   24   hour 
#>  3 2019-10-11   24   hour 
#>  4 2019-10-12   24   hour 
#>  5 2019-10-13   24   hour 
#>  6 2019-10-14   24   hour 
#>  7 2019-10-15   24   hour 
#>  8 2019-10-16   24   hour 
#>  9 2019-10-17   24   hour 
#> 10 2019-10-18   24   hour 
#> # … with 548 more rows

Created on 2022-09-15 with reprex v2.0.2

@peterdesmet
Copy link
Member

Why not a call the function get_effort() (get_custom_effort() behaviour) where you can group_by = "deployment" to replicate the current get_effort() behaviour?

@jimcasaer
Copy link
Collaborator

jimcasaer commented Sep 15, 2022

when looking at the list #171 of get_() functions, it seems to me that there are two families of get_() functions:

  • those that return information for each deployment and are afterwards used in map_dep()
  • the others that return information based on the export that is being analyzed

would it not be logical to keep the get_ as a prefix for all of them and differentiate using get_dep_() for the first group and get_projet_() or get_export_() for the second ? I do not like the suggestion to use summarize (as suggested in #173 ) given that these functions are for me not really summarizing something, but returning characteristics for each deployment

@peterdesmet
Copy link
Member

Suggested in camtraptor July 2023 coding sprint

Closing this issue in favour of #236

@PietrH PietrH closed this as not planned Won't fix, can't repro, duplicate, stale Sep 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants