Skip to content

Refined precision detection #29

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

Merged
merged 2 commits into from
Nov 14, 2014
Merged

Refined precision detection #29

merged 2 commits into from
Nov 14, 2014

Conversation

thiemowmde
Copy link
Contributor

The diff looks big because I renamed variables, made stuff private and reduced code duplication.

The main change is: The precision detection stops at a certain precision. Now there is a lower boundary, given by a maximum number of decimal places (after the comma). 8 for floats, 4 for DM/DMS (plus 2 places for the minutes and 2 for the seconds is also 8).

  • 0.00000001° is approx. 1 mm.
  • 1 / 36000000° is approx. 3 mm.

Reason for all this is IEEE. Some combinations of degrees and minutes turn into numbers like 1.20000000000001 and cause the detectors to return way to high precisions.

Bug: 64820

thiemowmde added a commit to wmde/DataValuesJavaScript that referenced this pull request Nov 6, 2014
@JeroenDeDauw
Copy link
Member

The diff looks big because I renamed variables, made stuff private and reduced code duplication.

Protip: make two commits, then we can look at the diff for each (even if its one PR)

@thiemowmde
Copy link
Contributor Author

Just some protectedprivate and some variable names. Not worth in this case, I think. Will try to do next time.

PS: Did it, it's two commits now.

tobijat added a commit that referenced this pull request Nov 14, 2014
Refined precision detection
@tobijat tobijat merged commit 015a5b5 into master Nov 14, 2014
@tobijat tobijat deleted the detection branch November 14, 2014 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants