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

Modify index.js to handle wrapped LAS files, meaning lines longer than 80 characters get continued on next line. #12

Closed
JustinGOSSES opened this issue Jun 25, 2019 · 7 comments
Labels
no-issue-activity question Further information is requested wontfix This will not be worked on

Comments

@JustinGOSSES
Copy link
Owner

Modify index.js to handle wrapped LAS files, meaning lines longer than 80 characters get continued on next line. Current wellio.js fails with LAS files that are wrapped.

https://github.com/JustinGOSSES/wellio.js/blob/master/dist/index.js

This was first suggested by @oze4 in issue #5.

This would be a good first issue and after a quick look seems like it would involve editing lines 158-194 in index.js.

Please refer to http://www.cwls.org/las/

@dcslagel
Copy link
Collaborator

dcslagel commented Nov 25, 2019

There is a test for this feature. It is currently disabled with '.skip'.

To run the test:

  1. Edit test_read_v2_sample_wrapped.js to remove '.skip'
  2. tape dist/test/las2json/test_read_v2_sample_wrapped.js

Note the test currently only tests that the las source file (with wrapped data) will be read without throwing an error. Once that is passed successfully additional test may need to be added to validate the data was parsed properly.

@dcslagel dcslagel self-assigned this Jan 21, 2020
@JustinGOSSES
Copy link
Owner Author

Should we unwrap the files by default as an initial preprocessing step?

Would that create any problems?

@dcslagel
Copy link
Collaborator

dcslagel commented Jan 22, 2020

@JustinGOSSES,

I am just now seeing your comment to look at unwrapping as a preprocessing step. I had been working on an alternative solution which is in pull request #31. See if it is also a satisfactory solution or if it needs changes.

Thanks,
DC

@dcslagel
Copy link
Collaborator

@JustinGOSSES

With pull request #31 merged, do we want to keep this ticket open for additional work in this area or can it be closed now?

Thanks,

DC

@JustinGOSSES
Copy link
Owner Author

JustinGOSSES commented Jan 29, 2020

I'll change this to "won't fix". ... want to keep it for now but not worked on.

@JustinGOSSES JustinGOSSES added wontfix This will not be worked on and removed enhancement New feature or request labels Jan 29, 2020
@JustinGOSSES JustinGOSSES removed the good first issue Good for newcomers label Feb 10, 2020
@dcslagel
Copy link
Collaborator

dcslagel commented Feb 10, 2020

@JustinGOSSES,

I'm wondering if we could handle this differently. Below is a proposal. See if it looks like a good way to go. If not, that's okay, but I thought I would run it by you anyways.

  • Move 'Unwrap the files by default as an initial preprocessing step' to a separate ticket.
  • Remove the 'wontfix' label and close this ticket.

The thinking for this proposal is that the integration of #31 did enable processing LAS files with wrapped lines which is this issue.

Thanks,

DC

@JustinGOSSES JustinGOSSES added the question Further information is requested label Feb 13, 2020
@dcslagel dcslagel removed their assignment Feb 20, 2020
@github-actions
Copy link

Stale issue message

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-issue-activity question Further information is requested wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants