Huh / collar

Utilities for exploring telemetry data
Other
7 stars 9 forks source link

Fixed errors in morph_gps function and updated for consistency #38

Closed foresthayes closed 5 years ago

foresthayes commented 5 years ago

Note that the cllr_add_id step was removed from the morph_gps function. I think that, if used, that function should precede the morph_gps function.

Huh commented 5 years ago

I documented it as an independent step in vignette. Are you using morph_gps? What do you see for its future?

foresthayes commented 5 years ago

I just switched to using morph_gps instead of just renaming and selecting columns manually. I could see morph_gps becoming a core function for the package (maybe eventually as an internal function?) i.e. all collar data is routed through morph gps to standardize the format and return a 'collar' class object. Thoughts?

Huh commented 5 years ago

I am happy if you like it. I will look it over later and comment further.

foresthayes commented 5 years ago

I am happy if you like it. I will look it over later and comment further.

I am not overly attached to the morph_gos, however I do think that it is important to provide an easy to standardize data sooner rather than later. If you have another direction in mind I'm happy to work towards that instead.

Does cllr_add_id work? Reading the diff it looks like it wouldn't work with just a mutate and no data.frame...

Good catch on the collar_add_id function. I'm not sure how I missed that, but yes it should have the data frame. I'll fix that shortly.

Is the date formate change to morph_gps too Vectronics centric? Will this still play nice with other companies?

Do you mean the default dt_format? I changed that to match ATS (Vectronics would need to include the "T"). Do you have any collars that provide it in the format "%d/%m/%Y %H:%M:%S"? I don't particularly care about the default but thought %Y-%m-%d is more common. Either way, the morph_gps function should return the dt column with a POSIXct class instead of a character.