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

On write, update step vars if curve idx changed #451

Conversation

dcslagel
Copy link
Collaborator

@dcslagel dcslagel commented Apr 17, 2021

Description:

This should complete Remove side-effects from write() #268.

This is a first iteration at updating the STEP variable as part of the write() process but only in the following cases:

  1. an initial curve index was not read from a las file (las.index_initial)
  2. or the curve index has changed during processing
  3. or if the STOP value doesn't match the final index value

I am not sure if case #3 should trigger updating the step variables. It looks like many of the test case LAS files have STOP values that don't match the final index value or the final index value - 1 STEP. Let me know if these needs to be modified.

Test Results:

Python 3.9

Name                       Stmts   Miss  Cover
----------------------------------------------
lasio/__init__.py             13      2    85%
lasio/convert_version.py      20     20     0%
lasio/defaults.py             11      0   100%
lasio/examples.py             42     10    76%
lasio/excel.py                88     34    61%
lasio/exceptions.py            6      0   100%
lasio/las.py                 431     60    86%
lasio/las_items.py           199     29    85%
lasio/las_version.py          50     14    72%
lasio/reader.py              396     25    94%
lasio/writer.py              166     10    94%
----------------------------------------------
TOTAL                       1422    204    86%

--

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

Thank you,
DC

@kinverarity1
Copy link
Owner

    • STEP should definitely be auto-calculated if LASFile is created from scratch.
  1. I'll look at the failing tests tonight but think that we can safely change them as long as the expected output makes sense...

@dcslagel dcslagel force-pushed the on-write-update-steps-if-index-changed branch from fb980fc to 59dc7e8 Compare April 20, 2021 02:28
@dcslagel dcslagel force-pushed the on-write-update-steps-if-index-changed branch from 59dc7e8 to ce6fbde Compare April 20, 2021 02:42
@dcslagel dcslagel marked this pull request as ready for review April 20, 2021 02:48
@dcslagel dcslagel requested a review from kinverarity1 April 20, 2021 02:48
Copy link
Owner

@kinverarity1 kinverarity1 left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

@kinverarity1 kinverarity1 merged commit 4d2fcc3 into kinverarity1:master Apr 20, 2021
@dcslagel dcslagel deleted the on-write-update-steps-if-index-changed branch April 20, 2021 13:57
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