Add support for distances in nautical miles

[brian-from-fl]

Added the DistanceUnit.NAUTICALMILES enumeration label with the corresponding "nm" unit suffix.

Closes #5085

[s1monw]

cool stuff - can you sign the CLA so we can pull this in?

[brian-from-fl]

The CLA was submitted just before this pull request; I have the PDF file so it should be on its way.

[chilling]

Nice One! But I think we should name it "nmi" rather than "nm" to avoid conflicts with metric units.

[brian-from-fl]

I've seen "nmi" suggested, but for any distances related to aviation (distances between two points, maximum ranges, and so on), I've only seen "nm". For one of many examples, the ranges of the various Gulfstream jets are given in "nm":

http://www.gulfstream.com/careers/our_products.html

But I'm all for changing the second string to "mni" (unless DistanceUnit supports more than 2 strings... I didn't have time to explore that one after I thought of it). But geospatial distances and aircraft distances never confuse "nm" with nanometers! So both "nm" and "nmi" would seem to be useful; let the client decide which to use. Does that make sense?

[chilling]

Sure, within a specific field there won't be confusion. My concern is more about setting up more metric units. In this case nanometers and their naming. IMO we should have the "mi" suffix on miles and keep "m" for meters.

[nik9000]

With nm being the SI symbol for nanometer I'd go with nmi for nautical miles. A shame they share the symbol.

[chilling]

yes, indeed!

[brian-from-fl]

In that case, I wouldn't recommend pulling in this change. When I receive a feed that uses "nm" for nautical miles, as 100% of the aviation world uses, I'll need to front ES with my own converter. And in that case, I'll just convert it down to meters and the current DistanceUnit already supports it.

On the other hand, I can't imagine nanometers being useful for geo distances. Academics aside, a human hair is about 100,000 nanometers; is ES going to be used to search for the nearest bacterium at the end of hair number 21,456,234?

http://www.rense.com/general74/small.htm

[chilling]

I still think, supporting nautical miles is a good and reasonable idea. Also we're discussing measuring distances, a part of ES that should kept as general as possible. Maybe it doesn't make sense (yet) to talk about nanometer distances in the current context, but we should not spoil future applications of this.

In my opinion we should primarily support the SI units and their abbreviations and search for reasonable alternatives on non-SI units. Also, if we turn the argument and ask what the abbreviation nm stands for, I would say even people of the aviation world ponder on the related field.

But for this special kind of purpose it's possible to set a default unit separately by the unit option. By setting this to NAUTICMILES, no matter which name we finally agree on, a value without an explicit unit specified, will be a nauticmile. I guess this will replace your own converter.

[brian-from-fl]

The DistanceUnit is case-sensitive? Then how about "NM" (another accepted, though not common, abbreviation for nautical miles), and also "nmi"?

Nobody in aviation would ever doubt what "nm" stood for, or they wouldn't last long in aviation! But I fully understand your argument, and can envision that nanometers might be extremely useful for the medical and bioengineering fields, for example (just not for geo-distances, of course).

So in the interests of SI coexistence I propose "NM" and "nmi". And then, of course. "nauticalmiles" for consistency with the other supported enumerations. How does this sound?

[chilling]

I agree! This is a great idea!

[brian-from-fl]

I had to define NAUTICALMILES with "nmi" before MILES with "mi" because "mi" is a subset of "nmi" (same reason that METERS had to come last). But now the test case succeeds with both NM and nmi as suffix strings. If you think this is acceptable, I'll commit and push this change.

I might even try to rebase it into one commit message but not sure. I'm solidly booked for the next few days. But if you like this compromise and you think I should rebase then let me know. Thanks!

[chilling]

@brian-from-fl yes it needs to be defined before miles. I guess rebase is not necessary right now, but please try. Thanks.

[brian-from-fl]

I don't have my expert handy and there are only two comments; the most recent one is accurate and complete. I committed, and then pushed up to my nm-branch. Do I need to make another pull request?

[brian-from-fl]

By the way, both of these test cases passed! (After I fixed the order!!!!)

    assertThat(DistanceUnit.Distance.parseDistance("53nmi").unit, equalTo(DistanceUnit.NAUTICALMILES));
    assertThat(DistanceUnit.Distance.parseDistance("53NM").unit, equalTo(DistanceUnit.NAUTICALMILES));

[chilling]

That's great. You don't need to open another pull request but it would be kind, if you can squash both commits.

[chilling]

sorry, I forgot to ask you if you can insert the new unit into docs/reference/api-conventions.asciidoc into the distance unit section. Just to have it all in one commit.

[brian-from-fl]

Re: Rebase. A bit confusing to this git newbie. If I make the most recent commit message all-inclusive, is that OK?

Re: docs. How's this? "Nautical miles" is two words; Is this acceptable or do you have a better suggestion:

    Nautical_mile:: 'NM' or 'nmi' or 'nauticalmiles'

[chilling]

no worries. On docs just insert Nautical mile:: NM, nmiornauticalmiles``. On git checkout to the master brach, pull current head, checkout to your branch, rebase to master. After that just squash the three commits into one and force push.

[brian-from-fl]

I did the rebase and squash thing... went ok including the most recent commit message update. Then a push failed. Needed to pull, but then a merge conflict. After correcting that, the commit message is not what it was but... as I said, this is my very first day with git and GitHub. I hope it's ok as it stands. My apologies if it's worse than before.

[brian-from-fl]

Had to fix the asciidoc since I noticed it used backtics and not single quotes.

Then on review, I noticed that the TimeValue is missing ms (milliseconds). But that's for another day, I suppose?

[chilling]

Yes, I thing the timevalues are another story. on the current branch you're always able to override your last commit message by git commit --amend.

[chilling]

I guess you solved all the conflicts and you're on the latest master now? so look up the git log for the last sha1 id that's not associated with your changes and do git rebase -i $sha1$ and replace all pick with s. but not the one in the first line, run the tests again.

[s1monw]

given that these are the official symbols for nautic miles symbol M, NM or nmi I am +1 on this.

[costin]

+1 on using the International System of Units conventions; I'm not a fan of using upper vs lower case but these are the official names so we should support them as such.