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

Additional feature - Conversions dialog or elsewhere #5395

Closed
shadrackkibet opened this issue Jun 28, 2019 · 8 comments · Fixed by #5458
Closed

Additional feature - Conversions dialog or elsewhere #5395

shadrackkibet opened this issue Jun 28, 2019 · 8 comments · Fixed by #5458
Assignees
Milestone

Comments

@shadrackkibet
Copy link
Collaborator

shadrackkibet commented Jun 28, 2019

I have stations catalogue and I would like to map all stations at KMD. Below is a section of the data.

image

I would like to have a facility in R-Instat to convert Degrees and minutes + seconds(when available) to latitude and longitudes.

This is possible in R using dms2dd() function from biogeo package. Using str_extract() from stringr package I extract letters (N, S, E, W) and minutes of latitude or longitude from Minutes columns. This is already possible using Find Replace dialog . You need to know some regex to be able to give the correct pattern. How can this be made easy for users who are not familiar with Regex? Here is a section of result columns.

image

Another alternative is using measurements::conv_unit() but this doesn't deal with "E/W" "N/S" entries, it requires positive/negative degrees.

I suggest this as another option in the conversions dialog or we can have this as a special dialog under mapping section? @rdstern @dannyparsons what are your views?

@rdstern
Copy link
Collaborator

rdstern commented Jun 28, 2019 via email

@dannyparsons
Copy link
Contributor

We've discussed this with Shadrack and think the best approach is to write our own function so we can make a few additions to the dms2dd() function and also avoid having to add a whole package for a single function.

@shadrackkibet
Copy link
Collaborator Author


dms2dd <- function (dd, mm, ss, ns) {
  n <- match(toupper(ns), LETTERS)
  we1 <- n == 19 | n == 23
  sgn <- ifelse(we1 == T, -1, 1)
  mm1 <- ifelse(missing(mm), 0, mm)
  ss1 <- ifelse(missing(ss), 0, ss)
  if(missing(dd))  stop("degrees must be supplied")
  decdeg <- (dd + ((mm1 * 60) + (ss1))/3600) * sgn
  return(decdeg)
}

@dannyparsons here is the implementation of the R function. Any comments?

@shadrackkibet
Copy link
Collaborator Author

Here is the design.

image

@dannyparsons
Copy link
Contributor

dannyparsons commented Jul 29, 2019

Looks pretty good. Few small points:

  • We can add a check that ns only contains the right letters, currently I could have "y" and it wouldn't complain.
  • we1 == T can be replaced with we1, since it's already logical
  • instead of checking if mm and ss are missing I would have 0 as their defaults, then a check is not needed. That's then clearer to the user what is happening
  • it would be good to have a slightly different name for the function than dms2dd to avoid a clash
  • I think you want mm1 / 60 instead of *
  • We can have a check that mm and ss are between 0 and 60
  • If ns is provided then dd must be positive (I think that's right?) as another check.

I think adding these checks are important because its good quality control of the type of data that will be used for this.

Is it also common to not have the N/S E/W column and instead the values are already positive/negative? If so, ns can be optional.

@shadrackkibet
Copy link
Collaborator Author

shadrackkibet commented Jul 29, 2019

Thanks, yes it is true that when ns is provided then dd should be positive. On your 5th point, I want / and not * because I have minutes converted to seconds.

convert_to_dec_deg <- function (dd, mm = 0 , ss = 0, ns) {
  n <- match(toupper(ns), LETTERS)
  if(!all(toupper(ns) %in% c("E","W","N","S"))) stop("ns must be one of E, W, N or S")
  if(missing(dd))  stop("dd must be supplied")
  if(!all(mm %in% 0:60)|!all(ss %in% 0:60)) stop("mm and ss must be between 0 and 60")
  if(!missing(ns)&any(dd<0)) stop("dd must be positive") 
  we1 <- n == 19 | n == 23
  sgn <- ifelse(we1, -1, 1)
  decdeg <- (dd + ((mm * 60) + (ss))/3600) * sgn
  return(decdeg)
}

I am not sure if it is common to have N/S E/W column. I think if this is not available then dd has positives\negatives. What do you think, should we also add a check if dd has positives and negatives then ns isn't needed?

I think we should also check that dd, mm, ss and ns are of the same length. I suggest we give a warning instead of stopping in this case.

@dannyparsons
Copy link
Contributor

You don't want %in% 0:60 because that only allows integers.
I've simplified a bit. I don't think we get too complicated with the checks, if dd has negative values and dir is also supplied they should go back and decide what they want to do. I've modified it so that dir is not required. We don't have to use this if we don't want to.
A length check would be good but if any are length 1 then that shouldn't get any message, otherwise it should give an error since we can't calculate with different lengths.
We should also check it works with missing values - if any component is missing the result is missing.

convert_to_dec_deg <- function (dd, mm = 0 , ss = 0, dir) {
  if(missing(dd))  stop("dd must be supplied")
  if(!missing(dir)) {
    dir <- toupper(dir)
    if(!all(na.omit(dir) %in% c("E", "W", "N", "S"))) stop("dir must only contain direction letters E, W, N or S")
  if(any(dd < 0)) stop("dd must be positive if dir is supplied") 
  }
  if(!all(mm >= 0 & mm <= 60, na.rm = TRUE)) stop("mm must be between 0 and 60")
  if(!all(ss >= 0 & ss <= 60, na.rm = TRUE)) stop("ss must be between 0 and 60")
  sgn <- ifelse(is.na(dir), NA, ifelse(dir %in% c("S", "W"), -1, 1))
  decdeg <- (dd + ((mm * 60) + ss)/3600) * sgn
  return(decdeg)
}

@shadrackkibet
Copy link
Collaborator Author

Thanks.

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

Successfully merging a pull request may close this issue.

3 participants