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

Fixed a bug with leading whitespaces in WKT #42

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jlaamanen
Copy link

Hi!

Not sure if this package gets updated too often anymore, but here's my 2 cents for fixing #36 which we've been running into recently.

Seems that the bug was caused by a missing white() call in the beginning of coords() - confirmed that everything still seems to work in unit tests after the fix.

Here's the list of changes:

  • Added a white() call to fix Parser does not parse wkt's with whitespace after first '(' #36
  • Added a couple of unit test cases with leading whitespace characters
  • Installed browserify to devDependencies to make npm install work without global installations
    • Tracked down the version by watching for changes to wellknown.js - seems that the version previously used was somewhere between 9.0.0 and 11.2.0 (chose the latest)

@mjumbewu
Copy link

mjumbewu commented Feb 28, 2024

Any progress on getting this merged in? I just ran into the issue in a class that I'm teaching. (/cc @throberto)

mjumbewu added a commit to musa-5090-spring-2024/course-info that referenced this pull request Feb 28, 2024
- Create a JS function for parsing WKT points because ALL THE LIBRARIES DO IT WRONG (see mapbox/wellknown#42)
- Use the ESRI proj codes library, because Proj4JS only comes with 4326, 4265, and 3857 predefined
- Speed up the Python script A LOT by using the non-deprecated pyproj workflow
@throberto
Copy link

Any progress on getting this merged in? I just ran into the issue in a class that I'm teaching. (/cc @throberto)

this repository has not been updated for a long time... we have a successor for it (https://github.com/placemark/betterknown) maybe this problem has already been fixed there

hey @tmcw 😄

@tmcw
Copy link
Contributor

tmcw commented Feb 29, 2024

betterknown tolerates leading whitespace. And, y'all are probably aware, but I don't have commit for this repo because I haven't worked for Mapbox in a half-decade :)

@mjumbewu
Copy link

Awesome, thanks for the heads up about betterknown @throberto @tmcw !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Parser does not parse wkt's with whitespace after first '('
4 participants