Skip to content

Answers to corner cases #10

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

Closed
wants to merge 2 commits into from
Closed

Conversation

facorread
Copy link

These two commits respond to some shapefiles that contain the ** characters as well as possibly empty shapes. This proposal should be innocuous in all other shapefiles.

@riggsd
Copy link

riggsd commented Dec 4, 2014

The first commit from this PR, 49edb13, should not be merged upstream. It catches only the case where two '' characters are present. This would only apply if the field happened to have been defined with size=2. A field with size=5 would have the value '****' in the same (NULL) situation.

The PR I submitted today, #16 , handles this as a general case.

I cannot comment on the second commit in this PR other than to say that unrelated changes should probably more appropriately go into multiple PRs.

@facorread
Copy link
Author

OK

@karimbahgat
Copy link
Collaborator

Closing as the null values were fixed in #16.

As for the null shapes, not sure if I understand it correctly, but since null shapes have a type value of 0 I don't think they would ever end up in those parts of the code. But make a PR explaining more if still relevant.

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.

3 participants