openworm / tracker-commons

Compilation of information and code bases related to open-source trackers for C. elegans
11 stars 12 forks source link

Added degrees and radians as units to Scala reader/writer. #125

Closed Ichoran closed 7 years ago

Ichoran commented 7 years ago

Default unit is radians (better for math).

Allowed variants are "deg", "degree", "degrees", "rad", "radian", "radians" with the 3-character forms taking the short SI prefixes (e.g. "mrad") and the others taking the long form (e.g. "centidegrees").

Documentation is not updated to reflect these as this is a proposal rather than a final decision on how to support angular units.

Note that supporting revolutions would be easy also at least in the Scala code base.

Ichoran commented 7 years ago

Review by @MichaelCurrie

MichaelCurrie commented 7 years ago

Are we going to normalize all radians to the [0,2π) interval (or perhaps to [-π,π)?)

Ichoran commented 7 years ago

@MichaelCurrie - I would say no, for two reasons.

  1. We may have measures of cumulative angle where 41.55π is meaningful.
  2. It doesn't simplify things much since as soon as you start doing any math with radians you have to deal with the circular nature of it. For instance, -179.999 degrees is very close to 179.999.
MichaelCurrie commented 7 years ago

@Ichoran ok makes sense, I will not normalize in my code either.

Each line in the code diff summary makes semantic sense to me. Assuming this is compiling for you, I'm going to merge.