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

Flake8 maintenance: lasio/las.py - Option 2 #568

Merged

Conversation

dcslagel
Copy link
Collaborator

@dcslagel dcslagel commented Mar 10, 2023

Description:

This change covers flake8 changes for one of the main Lasio files: las.py
This is part of Flake8 maintenance for Lasio code #564

Notes!

This Pull Request is the 2nd of two options! Select one of the options and close the other.

This option (the 2nd option) resolves las.py:1187:101: E501 line too long (101 > 100 characters) by moving unused parameters to kwargs in the update_curves() method.

The first option is Flake8 maintenance: lasio/las.py - Option 1 #567
It resolved the same issue by removing unused parameters from the update_curves() method.

Changes made

  • las.py::update_curves params to kwargs
  • las.py move lambda into sort function
  • las.py remove un-needed excel imports
  • las.py fix empty 'except's
  • las.py remove unused imports, fix Exceptions, shorten lines.
Changes suggested by flake8.
las.py:14:1: F401 'sys' imported but unused
las.py:37:1: F401 '.las_items.HeaderItem' imported but unused
las.py:37:1: F401 '.las_items.SectionItems' imported but unused
las.py:37:1: F401 '.las_items.OrderedDict' imported but unused
las.py:243:101: E501 line too long (102 > 100 characters)
las.py:275:101: E501 line too long (105 > 100 characters)
las.py:412:101: E501 line too long (144 > 100 characters)
las.py:443:25: E722 do not use bare 'except'
las.py:460:29: E722 do not use bare 'except'
las.py:482:25: E722 do not use bare 'except'
las.py:489:101: E501 line too long (105 > 100 characters)
las.py:536:13: E266 too many leading '#' for block comment
las.py:537:13: E266 too many leading '#' for block comment
las.py:663:13: F401 'openpyxl' imported but unused
las.py:667:13: F401 '.excel.ExcelConverter' imported but unused
las.py:701:12: E713 test for membership should be 'not in'
las.py:1023:13: E713 test for membership should be 'not in'
las.py:1065:13: E731 do not assign a lambda expression, use a def
las.py:1187:101: E501 line too long (101 > 100 characters)
las.py:1247:21: E722 do not use bare 'except'

Test Results

These changes reduce the number of lines of code in las.py and reduce the number of missed statements.

----------------------------------------------
Name                       Stmts   Miss  Cover
----------------------------------------------
lasio/__init__.py             28      6    79%
lasio/convert_version.py      20     20     0%
lasio/defaults.py             12      0   100%
lasio/examples.py             42     10    76%
lasio/excel.py                88     34    61%
lasio/exceptions.py            6      0   100%
lasio/las.py                 488     57    88%
lasio/las_items.py           220     30    86%
lasio/las_version.py          69     23    67%
lasio/reader.py              470     19    96%
lasio/writer.py              200      9    96%
----------------------------------------------
TOTAL                       1643    208    87%
Coverage XML written to file coverage.xml

--------------------------------------------------- benchmark: 1 tests --------------------------------------------------
Name (time in ms)                 Min       Max      Mean  StdDev    Median     IQR  Outliers     OPS  Rounds  Iterations
-------------------------------------------------------------------------------------------------------------------------
test_read_v12_sample_big     123.9254  125.2377  124.5981  0.4123  124.6210  0.5108       3;0  8.0258       8           1
-------------------------------------------------------------------------------------------------------------------------

--

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

Thank you,
DC

- las.py::update_curves params to kwargs
- las.py move lambda into sort function
- las.py remove un-needed excel imports
- las.py fix empty 'except's
@dcslagel dcslagel requested a review from kinverarity1 March 10, 2023 00:12
@dcslagel dcslagel changed the title Flake8 maintenance: lasio/las.py Flake8 maintenance: lasio/las.py - Option 2 Mar 10, 2023
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.

Thanks! Not sure I agree with some of flake's opinions, but it's good to have it passing where possible! 👍🏻

lasio/las.py Show resolved Hide resolved
@dcslagel
Copy link
Collaborator Author

dcslagel commented Mar 10, 2023

Thanks! Not sure I agree with some of flake's opinions, but it's good to have it passing where possible! 👍🏻

If there are some flake opinions, that shouldn't be integrated (or discussed further), let me know. Sometimes there are ways to adjust the code that make everyone happy, and sometimes the code can be reverted to before-this-branch and we can tell flake to ignore a specific coding pattern. If we were to tell flake to ignore things, I would lean toward putting those instructions in a .flake8 file rather than sprinkling flake8 comments throughout the code...

@kinverarity1 kinverarity1 merged commit 718af75 into kinverarity1:main Mar 30, 2023
@kinverarity1
Copy link
Owner

Thanks so much for tackling this! And sorry about the delay. All good now.

@dcslagel
Copy link
Collaborator Author

Okay, Thanks!. If there are follow up changes or reversions you would like to see related to this pull request let me know and I'll make a pull-request based on the requested changes.

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