search

DOI-USGS / lake-temperature-model-prep

Pipeline #1
Other
6 stars 13 forks source link

Resolve duplicate unit conversions #272

Closed padilla410 closed 2 years ago

padilla410 commented 2 years ago

Related to closed issue #235. When double checking files after merging PR #268 I noticed that Sam added a parse_unit_functions script to 7a_temp_coop_munge. This results in a duplicate function

I propose the following steps to resolve this issue: ~1. Move the functions from Sam's script from 7a_temp_coop_munge to generic_utils.R. Sam's function will replace my conversion function convert_ft_to_m~

  1. Replace all instances of convert_ft_to_m with feet_to_meters
  2. Double check parsers for opportunities to use feet_to_meters and fahrenheit_to_celsius (especially, in parse_Bull_Shoals_Lake_DO_and_Temp - left over from #235).
  3. Move fahrenheit_to_celsius to generic_utils.R
  4. Remove parse_unit_functions.R from 7a_temp_munge/src/data_parsers
padilla410 commented 2 years ago

Assuming there are no major bugs hiding, this will take <<30 min.

jordansread commented 2 years ago

Good catch. That feet_to_meters function is going to be slightly different since it rounds the conversion to get 3.28 instead of the equivalent 3.28084.

I prefer your function because it is more accurate but that complicates things when replacing it.

padilla410 commented 2 years ago

It does, but convert_ft_to_m is pretty old (2018). I think making all parsers use feet_to_meters will add an hour, tops (there are only 20 files in src/data_parsers and I can check through the initial commit to quickly ID most of files. I will update the issue