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

Revert the commits for modifying the data index #6

Merged

Conversation

dcslagel
Copy link
Contributor

Description:

This pull-request addresses issue #5 Revisit index re-assignment when writing LAS file. It reverts commits related to index re-assignment.

The thinking for this pull-request is that it is better to accept dlisio's index curve and not make the assumption that another channel should be the index based on its name.

While the LAS 2.0 specification does indicate that the index curve should be named one of: 'DEPT', 'DEPTH', 'TIME', 'INDEX', many real world las files use a different name for the index.

A possible better solution is to give the users an option to rename the index curve. In that scenario the rename would need to be checked whether it already exists as the name of another curve. Consideration would need to be given to whether the rename should apply to all las files generated from a given Dlisio input or only specific ones.

As an alternate solution users can use the Lasio tool to read las files with alternative index names. The index can be renamed within Lasio if the user needs to las files to specifically meet the the LAS 2.0 spec.

Reverted commits:

a20d419 Merge pull request #3 from
eecec92 Add test for new move_valid_index_to_first_col()
67dc925 Fix lascheck report of invalid data index channel

Tests:

All tests pass

Name                          Stmts   Miss  Cover
-------------------------------------------------
func_dlis_to_las.py             111     67    40%

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

Thank you,
DC

This reverts commits
a20d419 Merge pull request aruss175#3 from
eecec92 Add test for new move_valid_index_to_first_col()
67dc925 Fix lascheck report of invalid data index channel
@dcslagel
Copy link
Contributor Author

Hi @aruss175,

Just checking in on this pull-request. Is this something you would be interested to pull into DLISIO_Notebooks? If not, It is okay to go ahead and close it.. Of if it needs additional improvements to be accepted let me know and I can make the updates.

Thank you!,
DC

@aruss175 aruss175 merged commit b5444d4 into aruss175:master Nov 16, 2020
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.

2 participants