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

Adding a function unit_format #46

Merged
merged 2 commits into from Jun 12, 2015

Conversation

Projects
None yet
2 participants
@ThierryO
Contributor

ThierryO commented Jul 23, 2014

This function allows users to add units to labels. It has a scale argument that is useful when the data is in another units.

unit_format(unit = "m")(c(1e3, 2e3)) gives c("1,000 m", "2,000 m")
unit_format(unit = "m", sep = "")(c(1e3, 2e3)) gives c("1,000m", "2,000m")
data in m, labels in km
unit_format(unit = "km", scale = 1e-3)(c(1e3, 2e3)) gives c("1 km", "2 km")
data in inch, labels in cm
unit_format(unit = "cm", scale = 2.54)(c(1, 2)) gives "2.54 cm" "5.08 cm"

ThierryO added some commits Jul 8, 2014

@hadley

This comment has been minimized.

Member

hadley commented Jun 10, 2015

I wonder if it would be worth allowing something like unit = "m", scale = "c" (with a SI unit scale lookup)

@ThierryO

This comment has been minimized.

Contributor

ThierryO commented Jun 11, 2015

What would the output of unit_format(unit = "m", scale = "c")(c(1e3, 2e3)) look like?

@hadley

This comment has been minimized.

Member

hadley commented Jun 11, 2015

I was thinking of it the other way round: unit_format("m", "k")(c(1e3, 2e3)) would give 1 km, 2 km. (Or with c it would be 100,000 cm, 200,000 cm)

@ThierryO

This comment has been minimized.

Contributor

ThierryO commented Jun 11, 2015

What if e.g. the data is in mm and you want to display them in cm? I would do unit_format(unit = "cm", scale = 0.1)(c(50, 100)) which returns c("5 cm", "10 cm"). Your suggestion would be to do unit_format(unit = "mm", scale = "c")? I think it would require a lot of parsing.

@hadley

This comment has been minimized.

Member

hadley commented Jun 11, 2015

How about:

if (is.character(scale)) {
  unit <- paste0(scale, unit)
  scale <- c(m = 1e-3, c = 1e-2, k = 1e3)[scale]
}

so you could use either an SI prefix or a multiplier?

@ThierryO

This comment has been minimized.

Contributor

ThierryO commented Jun 11, 2015

That would work if the value is a basic SI unit (meter, gram, ...). What if the value is not in a basic SI unit (millimeter, kilogram, ...) ?
We might introduce confusion since unit_format(unit = "cm", scale = 10e-2) assumes output is in centimeter and the input is two magnitude higher (in this case meter). One would get the same result with unit_format(unit = "m", scale = "c").

Maybe we can do something like unit_format(input.unit, output.unit, scale). Only two out of the three can be specified.
unit_format(input.unit = "m", scale = "c") meter -> centimeter
unit_format(output.unit = "cm", scale = 1e-2) meter -> centimeter
unit_format(input.unit = "m", output.unit = "cm") meter -> centimeter
unit_format(input.unit = "m", scale = 1e-2) meter -> centimeter
unit_format(input.unit = "hm", scale = "d") hectameter -> decimeter or hectameter to decameter?

some tricky things:

  • area's unless specified in ares (1 hectare is 100 are, 1 hm² is 10000 m²)
  • volumes
  • input with non SI units
@hadley

This comment has been minimized.

Member

hadley commented Jun 11, 2015

If it wasn't in a SI unit already, you wouldn't use the character argument to scale. But I think you're right that it's too complicated.

@hadley

This comment has been minimized.

Member

hadley commented Jun 11, 2015

Can you re-document with latest roxygen, merge/rebase and add a bullet to NEWS?

@ThierryO

This comment has been minimized.

Contributor

ThierryO commented Jun 11, 2015

I'll do that early next week.

@hadley

This comment has been minimized.

Member

hadley commented Jun 11, 2015

Is there any chance you could do it today or early tomorrow? I'd like to submit to CRAN on Friday?

If you don't have time, just let me know and I can make the changes myself.

@ThierryO

This comment has been minimized.

Contributor

ThierryO commented Jun 11, 2015

Sorry, I won't have the time to do it before Monday.

@hadley hadley merged commit 38c171c into r-lib:master Jun 12, 2015

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
@hadley

This comment has been minimized.

Member

hadley commented Jun 12, 2015

No problems - now merged!

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