Skip to content

round_coordinates() precision check is bypassed by negative decimal latitudes #223

Description

@northfox

When x$coordinatePrecision is not set, round_coordinates() derives the original precision from the number of decimals in deployments$latitude:

https://github.com/inbo/camtrapdp/blob/d61868c/R/round_coordinates.R#L105-L111

lat_digits = nchar(stringr::str_remove(.data$latitude, "^\\d*\\."))

For a negative decimal latitude, the leading - prevents the pattern from matching, so the full string is counted (-33.123 → 7 instead of 3). Since any negative latitude with a decimal point yields at least 4 characters and digits is at most 3, the "Can't round to equal or higher precision" check can never trigger for a dataset containing such a latitude.

Coordinates with many decimals are still rounded correctly. But if the coordinates already have the requested precision or less, the function silently proceeds: coordinates are unchanged, while coordinatePrecision is set and rounding uncertainty is added to coordinateUncertainty:

library(camtrapdp)
x <- example_dataset()
x$coordinatePrecision <- NULL

# Latitudes with 3 decimals: correctly refused
round_coordinates(x, 3)
#> Error in `round_coordinates()`:
#> ! Can't round to equal or higher precision:
#> ℹ Original precision: 3 digits (based on maximum number of decimals found in
#>   latitude in deployments).
#> ℹ Requested precision: 3 digits.

# Same data south of the equator: accepted
deployments(x)$latitude <- -deployments(x)$latitude
x_rounded <- round_coordinates(x, 3)
unique(deployments(x)$coordinateUncertainty)
#> [1] 187
unique(deployments(x_rounded)$coordinateUncertainty)
#> [1] 334

A related, rarer case: latitudes without decimals (e.g. 51) have no . to match, so their integer digits are counted (2 instead of 0). #104 noted that 1.000 should count as 0 digits; the pattern proposed there (^\\d*.) covered that case, but the current pattern (^\\d*\\.) does not.

The existing test for this precision check only uses positive decimal values (e.g. 4.1), which is why this wasn't caught.

Proposed fix: count only the characters after the decimal separator (0 when there is none), and add regression tests for negative and integer latitudes. If this sounds right, I'm happy to open a PR.

Notes / out of scope:

Setup: macOS 26.6.2, R 4.6.1, frictionless 1.3.0. Reproduced with camtrapdp 0.6.0 (CRAN) and 0.6.0.9000 (main at d61868c).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions