Skip to content
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

Lat / Long matches incorrectly #197

Closed
bee-san opened this issue Oct 9, 2021 · 6 comments · Fixed by #226
Closed

Lat / Long matches incorrectly #197

bee-san opened this issue Oct 9, 2021 · 6 comments · Fixed by #226
Labels
good first issue Good for newcomers hacktoberfest help wanted Extra attention is needed priority Regex issue Regex is not full or matches a lot of false positives

Comments

@bee-san
Copy link
Owner

bee-san commented Oct 9, 2021

$ poetry run pywhat acb6d73d95a10d30aef9894603e90963   

Matched on: e90963
Name: Latitude & Longitude Coordinates
Link:  https://www.google.com/maps/place/e90963

Bug found in #196

@bee-san bee-san added the help wanted Extra attention is needed label Oct 9, 2021
@ghost ghost added hacktoberfest good first issue Good for newcomers labels Oct 9, 2021
@Vipul-Bajaj
Copy link

Can you please explain more on this issue?

@bee-san
Copy link
Owner Author

bee-san commented Oct 9, 2021

Can you please explain more on this issue?

Sure! e90963 is not valid latitude / longitude coordinates and it shouldn't match :(

@Saacket
Copy link

Saacket commented Oct 17, 2021

plz explain

@bee-san
Copy link
Owner Author

bee-san commented Oct 17, 2021

plz explain

What are you confused about? :)

image

e90963 is not valid lat / long coordinates as it's only one single item (that's rather small)

@bee-san
Copy link
Owner Author

bee-san commented Oct 26, 2021

Another example:

Matched on: e26
Name: Latitude & Longitude Coordinates
Link:  https://www.google.com/maps/place/e26

@bee-san bee-san pinned this issue Oct 26, 2021
@nodtem66
Copy link
Contributor

nodtem66 commented Nov 3, 2021

I propose this new one: https://regex101.com/r/WrINyr/1

The old RegEx consists of three patterns:

  1. Full expression with symbols and prefix N/W/S/E , e.g. N 32° 53.733 W 096° 48.358
    (?:(?:N|W|S|E)\\s?\\d+\\s?\\u00B0?\\s?\\d+\\.?\\d*\\s?\\'?\\s?\\d*\\.?\\,?\\d*?\"?\\s?){1,2}
  2. Full expression with symbols and subfix N/W/S/E, e.g. 13°55'24.3"N 101°20'30.1"E
    (?:\\d+\\s?\\u00B0\\s?\\d+\\s?\\'\\s?\\d+\\.?\\,?\\d{0,}?\"\\s?(?:N|W|S|E)\\s?){1,2}
  3. Two decimal numbers without symbols and prefixs, e.g. 52.6169586, -1.9779857
    (?:[-+]?(?:[0-8]?\\d+\\.\\d{4,}|90(?:\\.0+)?),\\s*[-+]?(?:180(?:\\.0+)?|(?:(?:1[0-7]\\d)|(?:[1-9]?\\d))(?:\\.\\d+)?))
    "Regex": "(?i)^((?:(?:N|W|S|E)\\s?\\d+\\s?\\u00B0?\\s?\\d+\\.?\\d*\\s?\\'?\\s?\\d*\\.?\\,?\\d*?\"?\\s?){1,2}|(?:\\d+\\s?\\u00B0\\s?\\d+\\s?\\'\\s?\\d+\\.?\\,?\\d{0,}?\"\\s?(?:N|W|S|E)\\s?){1,2}|(?:[-+]?(?:[0-8]?\\d+\\.\\d{4,}|90(?:\\.0+)?),\\s*[-+]?(?:180(?:\\.0+)?|(?:(?:1[0-7]\\d)|(?:[1-9]?\\d))(?:\\.\\d+)?)))$",

The first pattern slightly differs from the second pattern in the symbol matchings

  • \\u00B0? and \\u00B0
  • \\'? and \\'
  • \"? and \"
    This causes the RegEx matching of e26, e90963, and so on.

I've observed that there is nothing like N 52.6169586, W -1.9779857; only N 32° 53.733 W 096° 48.358, and 52.6169586, -1.9779857 are valid.

Then these symbol matching should've been changed to the strict matching of \\u00B0, \\', and \" or the flexible matching of \\u00B0, \\'?, and \"?.

If it looks fine, I'll make a patch.
Thanks

@ghost ghost added the Regex issue Regex is not full or matches a lot of false positives label Nov 3, 2021
nodtem66 added a commit to nodtem66/pyWhat that referenced this issue Nov 3, 2021
bee-san added a commit that referenced this issue Nov 7, 2021
Patch #197 Lat / Long matches incorrectly
@bee-san bee-san unpinned this issue Nov 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers hacktoberfest help wanted Extra attention is needed priority Regex issue Regex is not full or matches a lot of false positives
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants