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

Add temporal plot of deployement #164

Open
Rafnuss opened this issue Sep 9, 2022 · 9 comments
Open

Add temporal plot of deployement #164

Rafnuss opened this issue Sep 9, 2022 · 9 comments
Labels
enhancement New feature or request
Milestone

Comments

@Rafnuss
Copy link
Collaborator

Rafnuss commented Sep 9, 2022

I've been using this plot to check the date of my deployements and the corresponding image to check that everything was correct (eg. camera running all the way to the end of the deployement, etc).
MicrosoftTeams-image

I thought it might be a nice plot to add to the package.
I was thinking of following the argument than map_dep() roughly.

  • What function name? time_dep ?
  • x axis would be time, but y axis could be either deployement or camera. Camera would allow to have smaller y axis and also check for overlaping camera-deployement error. What do you think? As a parameter?
  • feature would be the same as map_dep(): n_individuals, n_species, n_obs. Is there a reason for not setting one as default ?
  • I guess I can also do the filtering by species, sex, life_stage. I need to check the code how this is done.
  • library: I guess ggplot2?
  • plot style: scatter plot would be the easier, but only work for n_obs (maybe n_individuals is taking the median/mean value). Otherwise, I could aggregate the data per time (e.g. day) and do a matrix plot (or scatter plot with square symbol). Any advice?
@damianooldoni
Copy link
Member

I think this functionality fits perfectly in this package, @Rafnuss! Thanks.

Function name

time_dep() seems to me a good name as well. But if it goes about naming things, I always ask an opnion from @peterdesmet 😄

Axis

x-axis: timestamp
y-axis: I would use by default locationName which is at researcher level the most readable field. But it can be set to one of the following deployment columns as well:

  • deploymentID
  • locationID

feature

yes, same features, please, but only the ones you think make sense. Still, we would like to change the way of working of map_dep(), see #91. The same workflow would be applied to this function as well. I hope to find time for #91 so that it is ready before end October.

filtering

Again, this is also something we would like to do before hand, see issue #92. In this way, both map_dep() and time_dep() will not have to implement any extra filtering.

library

yes, ggplot2.

plot style

Scatter plot is an option, yes. Still, I was thinking about an heatmap: it would improve the appeal of the plot, I think. I was wondering what you thought about matrix plot, maybe an heat map?

@damianooldoni damianooldoni added the enhancement New feature or request label Sep 13, 2022
@peterdesmet
Copy link
Member

Naming: ideally the first part of the function name is a verb, so time_ doesn't work here. It could be plot_dep but that doesn't convey you're plotting time information. Another option would be to name all visualization functions (currently this one and map_dep) plot_x() or viz_x(). So I think my preference would go to:

  • plot_dep_time()
  • plot_dep_location() (renamed map_dep())

@damianooldoni
Copy link
Member

Thanks @peterdesmet! You are right. The point is that map_dep() returns a (leaflet) map, not a plot.
IMO, I find that plot_dep_location() is also quite long.

I think viz_ prefix is better:

  • `viz_dep()´
  • viz_dep_time()

I would use viz_dep() instead of viz_dep_location() simply because I associate deployment visualization to the geographical information associated to such deployments.

@peterdesmet, @Rafnuss: could you follow my reasoning?

@Rafnuss
Copy link
Collaborator Author

Rafnuss commented Sep 13, 2022

Thanks for the input. Yes, I follow the logic and this could work well. I do have a slightly difference option to suggest.

  • I would personally remove the _dep_. map_dep() could be plotting either the observation or the deployment, it would be the same while the time() function shows both observations and deployments.
  • I do like having plot in front as it provides a very user friendly reading of the different functions. From my perspective,viz are more of a complex/multi figure/plot or infographic style. But I'm open to viz.
  • IMO, leaflet map are a sort of plot (just with a map background = (geo-)graphical information). I understand the technical difference, but we could also use ggplotly to also create html/js visual.

So, I would go for plot_location() or plot_map() or something. (Would it be possible to actually use leaflet directly on the data: mica %>% get_n_obs() %>% leaflet()). And then plot_time().

But I don't have much experience, so happy to follow your advice.

@jimcasaer
Copy link
Collaborator

I'm strong in favor of using the leaflet function and would distinguish these functions using leaflet from those returning plots
this would give you within camtraptor a family of functions map_() and plot_()

I agree that we should think about the second part of the function names given that a deployment returns many different interesting variables (n_obs, n_species, n_indiv, ...)

@peterdesmet
Copy link
Member

I agree that it would be better to remove _dep_ from the function name, especially if all map/plot functions would work on the package property (which may or may not be filtered beforehand by one of the filter() functions, see #92). So, I'm in favour of plot_time(). I'd have to reassess the other function names.

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

Decided to keep plot_ and map_ as separate verbs and functions. @Rafnuss the function you suggest in this issue should be named plot_time(). Renaming map_dep() (if necessary) would happen at a later stage.

@Rafnuss
Copy link
Collaborator Author

Rafnuss commented Nov 26, 2022

Hey, I'm a little squeezed with time at the moment and don't think I'll be able to develop this functions now (and for the next 6-9months I belive to be honest with you). Sorry. Is it better to close this issue for now?

@damianooldoni
Copy link
Member

Hi @Rafnuss. You don't need to say sorry! I would prefer to leave the issue open as it can be that I will work on this next year. Showing this kind of information is quite important. In case I work on it, would you like to be (one of) the reviewer(s)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants