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

Handle duplicate headers and changing number of data columns. #71

Conversation

dcslagel
Copy link
Collaborator

@dcslagel dcslagel commented Mar 31, 2021

Description:

Handle duplicate headers and changing number of data columns.
This pull-request enables parsing some of the LAS files in issue:

Changes to dist/index.js:

  • Only read a required unique header and its data once.
    Ignore additional entries for the unique header.
    These are the headers: ~V, ~W, ~C, ~P, ~O, and ~A.
  • Move the inital parsing from las string data in interim structured
    section data to sub-function: las2json.readSections()
  • Move las2json sub-functions to the bottom of the section to make the
    main logic more prominent.
  • Handle cases of data sections that have an inconsistent number of data
    columns in the data rows. If a row has more data columns than the
    headings, then add generic colunms titled 'UNKNOWN' + a number. Then
    populate the previous rows for that column with zero string values '0'

Add tests:

  • dist/test/las2json/test_duplicate_header.js
  • dist/test/las2json/test_extra_data_columns.js

This is a fairly big change, with moving some code sections around and replacing some code with a sub-function.

Test Results:

npm run test-las2json
...
1..50
# tests 50
# pass  50

--

Let me know if this change could be accepted (or rejected) or
needs some additional changes to be approved and merged.

Thank you,
DC

- Only read a required unique header and its data once.
  Ignore additional entries for the unique header.
  These are the headers: ~V, ~W, ~C, ~P, ~O, and ~A.
- Move the inital parsing from las string data in interim structured
  section data to sub-function: las2json.readSections()
- Move las2json sub-functions to the bottom of the section to make the
  main logic more prominent
@dcslagel dcslagel changed the title Handle duplicate required unique headers Handle duplicate headers and changing number of data columns. Apr 2, 2021
- Handle cases of data sections that have an inconsistent number of data
columns in the data rows. If a row has more data columns than the
headings, then add generic colunms titled 'COL' + a number. Then
populate the previous rows for that column with zero string values '0'.
- Add test case: las2json/test_extra_data_columns.js
@dcslagel dcslagel force-pushed the handle-duplicate-require-unique-headers branch from 85f7811 to 94cc371 Compare April 2, 2021 15:06
@JustinGOSSES
Copy link
Owner

Sorry I've been slow on this one. Will try to review this within the week. As you say, it's a bigger change than normal, so want to give it a good review.

@JustinGOSSES JustinGOSSES self-assigned this May 22, 2021
Copy link
Owner

@JustinGOSSES JustinGOSSES left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Read through it and ran all tests.

Unfortunately, I'm left this sit too long and now there's some conflicts with newer pull requests. Hence, I've left some old code in but commented it out. Will go back in and delete the commented out code after a while if no problems in use as well.

@JustinGOSSES JustinGOSSES merged commit 82a4def into JustinGOSSES:master Jun 20, 2021
@dcslagel dcslagel deleted the handle-duplicate-require-unique-headers branch June 20, 2021 22:08
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.

None yet

2 participants