Update specification

[fulldecent]

This PR creates implementable specifications for Plus Codes and Open Location Codes with minimal changes to what is currently published.

And it removes all contradictory information in this project.

In the process, this solves a majority of open issues in one pull request.

Because of contradictions in the project as it, it is NOT possible to make this into bite-sized PRs. I appreciate your time reviewing this thoroughly.

Normative changes

• Define codes for north and south poles
• Specify which code levels of full codes may be shortened to which code levels of short codes
• Specify which API methods are REQUIRED or OPTIONAL
• Establish and define naming (Plus Codes, area, bounds, long code, etc.) for consistent usage
• Define which full code a short code refers to
• Move any authoritative normative statements from wiki and other places here into the specifications
  • wiki/Naming-guidelines
  • FAQ.txt
  • olc_definition.adoc
  • specification.md
  • CHANGELOG.md
  • Guidance-for-shortening-codes.md
  • README.md / "Each implementation provides..." notes

Other chages

• Removed duplicative/outdated specifications and "recommendations", updated links
• Moved all documentation into Documentation folder, updated links
• Basic copyediting and formatting on files I touched
• Migrated documentation consistently to Markdown format
• Updated all references to use "Plus Codes" when codes are being referred to (implementations and systems are still "Open Location Code"), fixes Plus Codes capitalization inconsistent with marketing website
• Move implementations into Implementations folder, delete non-implementations, reference everything in README with note of new API compliance
• Added test cases to match normative requirements
• Adds notes about which platforms will produce wrong results because they have insufficient number precision (useful for embedded applications)

Fixed issues

• "Global code" may refer to a 10 or 11 digit code
  • Because references to the outdated "global code" and regex are gone
• OLC inconsistent reference to Earth equatorial circumference
  • Because equatorial distance is no longer referenced, and more approximate distances are given
• Specify a unique representation for North and South poles
  • Because this is specified in the Area and bounds section
• Specification should recommend function names
  • Because adds recommended method names
• Documentation conflicts on whether 8 digits may be removed in shortening
  • Because this is specified as 2, 4, or 6 characters are removed
• Plus Codes regex is incorrect
  • Because new correct regexes are given
• OLC inconsistent reference to Earth equatorial circumference
  • Because no circumference is referenced and values in meters are approximated slightly
• OLC specification doesn't strictly state validity status of codes with odd length
  • Because specification is given on code lengths
• Short codes with padding are unintentionally considered valid
  • Because specification is given for short codes
• Google maps and 'Find your code' tool create short codes which aren't decodable offline
  • Beacuse invalid marketing claims are removed
• Test for efficient implementation by fulldecent
  • Because tests are added and implementations are required to implement correctly.

Makes progress on these issues

• Plus Codes capitalization inconsistent with marketing website
  • Because it uses and specifies the correct capitalization of "Plus Codes". But the implementations also need to be updated.
• Floating point representation errors may cause incorrect codes
  • Because detailed discussion includes notes about floating point errors and it includes a recommendation implementation that produces correct results. It also references classes of floating point errors that are not discussed in this issue. But the implementations also need to be updated.
• Use consistent naming in this repository -- plus codes or open location code
  • Because the documentation is updated to make consistent and correct usage of Plus Code versus Open Location Code. Implementations are not updated. I hope this PR can please be reviewed and merged before I consider the implementations.

⭐ Requested action

• Merge this pull request

• Update project wiki per commit https://github.com/fulldecent/open-location-code-wiki/pull/1/files. Project maintainer can do this with:

  1. Clone git@github.com:google/open-location-code.wiki.git
  2. Pull from https://github.com/fulldecent/open-location-code-wiki
  3. git log and/or git diff HEAD~ to review changes
  4. Push

• Tag and release this update at https://github.com/google/open-location-code/releases

Follow-on work

After this PR is merged, I can:

• Create reference implementation, validate and fix one existing implementation and note is as verified compliant
• Move implementations to Implementations/ folder (also update README, .travis.yml)
• Delete non-implementations (just delete file, already properly referenced in README)
• Move demos to Demos/ folder
• Losslessly compress images

And I can help but might need help with:

• Create example implementation for embedded applications in 32 bits and 16 bits

[fulldecent]

This PR includes a move of all implementations into the Implementations/ folder and this inflates to a scary high Files Changed number. Please review this PR using VS code and you will see there are fewer actual changes. This file moving is required for this PR because it distinguishes the "implementations" from the "demos". And implementations now have a full specification.

I have worked hard to make this a "bite sized" and "minimal" PR.

[fulldecent]

Requesting PR review, please, from @bilst, @bocops, @drinckes

---

Specifically, requesting help to check:

• Off-by-one error. Please see Plus Codes Specification where the area is defined. Other than the north and south poles, I have (mostly) just simply used existing specifications and used different wording. For example I have defined the code 33000000+ as including inside the square 70S,160W..50S,140W, the exact SE point 70S,140W and the southern and eastern borders of that square, but not including the NE corner. Is this the intended definition, or should I change it for southern and western borders and SW corner?

(Update on 2021-06-07, this ^^ specific item is fixed at 2ce7dca and confirmed to match behavior of existing C implementation.)

[bocops]

I haven't had time to check all of this in detail. The fact that this is a massive change that also moves around unrelated parts makes it hard to do so - and considering that even much smaller PR currently aren't accepted quickly makes me wonder if this repo and its maintainers and contributors are even willing and able to handle a PR of this scope at the moment.

For what it's worth, reading through the rewritten Plus Codes Specification.md, the whole thing feels much less readable and understandable to me. While more strict specifications are appreciated in some places, what this now lacks is a good overview of what a plus code even is, what it looks like, what it can be used for, without immediately jumping into "SHALL NOT", "MUST", and "MAY" legalese.

On another note, the rewritten specs claim to be v1.0.5, a patch-level bump from the previous 1.0.4, but I immediately see some areas where there are larger (if not even breaking) changes to the specification. This means that this new specification, if accepted, should be 1.1.0 or even 2.0.0, which in turn means that there would need to be an even more thorough look at it (and reason for the change in the first place).

To give just one example for a potentially breaking change, areas around the poles are now strictly defined to include or not include the poles themselves. In the case of the south pole, where before all areas having the pole as one of their corner points would include it, now only one of them (for any given code length) does. On top of that, the area that does is the one east of the antimeridian, not the one east of the prime meridian, although the latter ("0°") is typically used when giving coordinates of the poles. While this might look like a rather unimportant detail, it can make the resulting code worse than strictly necessary, as has been laid out in #450.

[fulldecent]

👍 Thank you for looking at my PR, I do really appreciate it.

Poles

I agree that all details here are important and I commit to fix everything to earn acceptance of this PR. Yes, prime meridian is a more customary choice.

✅ Fixed at 8345cf4

Introduction and readability

The current Plus Codes specification also does not include an introduction.

And this PR does include the same introduction file as before.

Is this the file you were looking for?

Here is a potential brief addition at the top of the Plus Codes specification (above the version history):

In much of the world, street addresses are poorly defined or non-existent. Plus Codes allow to encode location information into easily exchangeable codes. For example, 8FVC2200+ includes the area around Romoos, Switzerland. Shorter and longer codes can represent various area sizes, are helpful for navigation and are very convenient for humans to remember and use.

❓ Shall I make that addition?

Massive changes

The PR currently moves all implementations into an Implementations/ folder. This is in contrast to Demos which go to the Demos/ folder. This is significant because the Open Location API Specification applies to "implementations".

❓ Shall I factor out this change to discuss separately?

Version number

You are referring to Semantic Versioning. Thank you, I agree 1.1.0, at a minimum, is appropriate.

🦶 Updated at d7a7a91.

❓ Shall I change this 2.0.0?

Corners and edges

I have corrected the specification to include the south-west corner of each area. I tested the C implementation works this way and also added test cases.

✅ Corrected in 2ce7dca

Other notes

I am not deterred that other PRs are not accepted quickly.

This project "strongly encourages technical contributions" and I hope this significant technical PR, pushed by somebody committed to see it through, and flexible to get it done, can see the light of day. I am also around to take on more responsibility later when I know good PRs are actively reviewed and acted on. (Specifically, I'd like to update one of the implementations to be a "reference implementation" as a great starting example.)

[bocops]

Re: Introduction/Readability ("Shall I make that addition?"): Perhaps I've just been confused by the amount of change. Let's revisit this at a later point, once all the other pieces have fallen into place, wherever that might be?

Re: Massive changes ("Shall I factor out this change to discuss separately?"): I actually appreciate the enhanced structure of having all implementations but not the random rest in one folder, but I think this might best be handled separately, if only to remove some of the noise from this PR.

Re: Version number: You're right, this repo doesn't explicitly state that it does use semantic versioning, but I think that is the reasonable choice. Whether a minor version bump would be sufficient in my opinion depends (probably among other things) on whether decoding a plus code encoded with 1.0.4 using these new specifications still leads to the same coordinates in all instances. Perhaps let's identify those changes first that would mandate a major version bump, and see which ones are considered absolutely necessary?

[fulldecent]

Thank you for all the notes. Below is a summary of (recently) resolved and (every) remaining items.

Poles and breaking changes

Breaking change warning added.

✅ Fixed at ffe0a3a

Required API methods

Also I moved isValid, which regards full+short codes from required to optional. This is consistent with all other things relating to short codes being optional. Separately, no implementation implements isValid currently as specified (see below).

Breaking change warning added.

✅ Fixed at ce6bb6d

Replaced specifications:

• https://github.com/google/open-location-code/blob/main/API.txt
• https://github.com/google/open-location-code/blob/a323e80159d6d6170158cc22497250cb86bf3583/docs/specification.md#library-implementation-requirements

Regex

Agreed, there is a slight difference in the isFull, isShort, isValid methods. Now they actually must validate the Plus Codes. This is easy because a PCRE implements this exactly.

No implementations implement the current required API. Specifically, they all check for the format separator (+) which is not part of the spec.

The old regexes were wrong and/or conflicting and removed.

The new Regexes exactly match valid Plus Codes, the spec, and the plain English meaning of the method name "is valid".

Breaking change warning added.

✅ Fixed at daa1c35

Copyright

Thank you! Deleted.

✅ Fixed at d7c6d25

Code length lookup

✅ Fixed at a2b4168

"Precision"

A more specific wording describes the geometry of the Plus Code area rather than the unclear word "precision".

✅ Fixed at 33044f5

Earth

Normative reference to Earth is removed.

✅ 79ca2c2

Open items:

• Which methods should be required? Update specification #463 (comment) // and should supporting all code lengths (not just length 10) be required?
• Short codes
  • Add test cases
  • Add a margin of safety
    • Update specification #463 (comment)
    • Update specification #463 (comment)
  • If geocoding, recomend to use city names // Update specification #463 (comment)

[fulldecent]

@bocops Could you please share any thoughts on the open items immediately above?

[bocops]

Sorry for the late reply. :)

Regarding required vs. optional methods, I think we agree that all current functionality regarding the handling of full codes needs to stay required. According to current API.txt, that is: isFull, encode, decode

I'm not sure that making all functionality related to short codes completely optional is the correct thing to do. At the very least, contributors should be strongly encouraged to offer this functionality in their implementations. Whether we decide for or against this change, this would affect these methods: isShort, shorten, recoverNearest

The remaining isValid is a special case. Even if we decide to make short code handling optional, a short code is still a valid code, so if this method name stays, it should behave the same across all implementations, independent of whether a certain implementation does or doesn't handle short codes. At that point, discussing the removal of shorten-related functionality becomes somewhat moot.

In the end, because of this, and because the main problem with short codes seems to be not their existence but the fact that the main publisher of short codes chooses reference locations rather opaquely, I believe that all seven methods might as well stay required.

Regarding the question of required code length support, I assume this means full code lengths outside of shortening? A while ago, we had a lengthy discussion about what code lengths are sensible in the first place, and the decision was made to restrict code length to <=15, because anything beyond that doesn't really have any real world use cases.

An argument could be made that codes referencing an area of <= 10cm² (code length 14, 15) aren't that sensible in practice, either - but unless those fall into the range of "technically impossible on specific systems", I don't think we should make some lengths required but others optional. If there are precision problems, we should rather define how an implementation should deal with them - for example, should decoding a "too long" code simply fail, or should it return the next-bigger area?

Regarding safety margins when shortening, I think the best description so far can be found on the wiki page Guidance for shortening codes, although I'd prefer to not simply hand out magic numbers like 0.4, but explain a bit better why we're not going for the theoretical full range of 0.5.

[fulldecent]

I am considering to update the PR and make it:

• All methods required. This is simpler and it restricts which implementations we link to/develop in this repo to only be the highest quality implementations. If somebody wants to make a lesser implementation, it will be up to THEM to explain why they chose to exclude short codes or something else.
• Safety. Clearly specify safety margins. Make this a RECOMMENDATION.
• Required code length. Like you say, only specify how it should work and how it should gracefully degrade. Explain why.

If I implement all these things, does that clear the threshold for merging this PR?

[fulldecent]

@bocops @drinckes Can you please confirm that if I make the changes above #463 (comment) and create a reference implementation that this PR may be merged?

[bocops]

@fulldecent This is a decision for @drinckes or potentially @bilst to make. I'm still not convinced that all of the breaking changes are really necessary, but in the end this is their project. Either way, I agree it would be nice to see some closure here, this has gone on long enough.

[fulldecent]

@drinckes @bilst, please advise.

I'd like to update these three things if you think it will be mergable.

After merged I will work on a reference implementation.

[bilst]

Sorry for the slow reply! I think you point out some interesting theoretically valid technical issues, but given that they haven't been an actual problem for any users, I'm not sure it's worth the added complexity/formality to call them out specifically.

Easiest PRs to accept are small scope improvements to clarity (e.g. editing a paragraph or two in a single file). Any increased complexity of documentation or added requirements need to be weighed carefully against their potential to discourage adoption by GIS folks who might not be as comfortable working with more formal specifications.

[fulldecent]

@bilst

issues... haven't been an actual problem for any users

Instead of considering the problems of users (which is super limited, because few people use Plus Codes), please consider the problems of potential users.

There are many people that don't use Plus Codes. Perhaps this is because it has problems and it is not possible to correctly implement Plus Codes.

People that develop products (even GIS folks) like correct specifications and example code. On the other hand, people that start developing products, but never finish... those are the people that like vague and inconsistent specifications.

This PR addresses these issues, holds Plus Codes in the highest light, and lays the path forward for multiple compatible and correct implementations.

---

I will not break this PR up into smaller pieces because too many things are broken as-is.

You are welcome to close this PR without merging if you still do not have the time to provide a proper review.

[drinckes]

I'm not even going to try to understand a single PR of this size.

For future reference, I would rather have multiple (i.e. small) PRs for a single issue, than one PR that attempts to address multiple issues.