sixty-north / segpy

A Python package for reading and writing SEG Y files.
Other
99 stars 54 forks source link

Moved IBM float un/packing routines out of toolkit.py into a new module. #84

Closed abingham closed 6 years ago

abingham commented 6 years ago

The main thing this does is pull all of the IBM float un/packing code out of toolkit.py into ibm_float_packer.py. This should make it easier to get toolkit.py coverage up to 100% by deconflating it with the techniques we use for detecting the C++ IBM float routines.

Full test coverage for the new module will still be tricky, though. As currently implemented, there will always be at least some part that can't be tested. This is because the C++ modules either can or cannot be imported; we can't have it both ways in a single test.

An alternative implementation using plugins (theme for the day!) might be able to get to 100% coverage if that's a goal we really want to achieve.

coveralls commented 6 years ago

Coverage Status

Coverage remained the same at ?% when pulling 784748739d52c700a3ff1ed74567efb87527bc8e on abingham:float-packing-refactor into 8b2f89cffaeb5cb1010b828d2be06fca02e58311 on sixty-north:master.

rob-smallshire commented 6 years ago

I think plug-ins is an excellent choice. We can use setuptools entry points, can’t we?

On Tue, 6 Feb 2018 at 11:27, Austin Bingham notifications@github.com wrote:

@abingham https://github.com/abingham requested your review on: sixty-north/segpy#84 https://github.com/sixty-north/segpy/pull/84 Moved IBM float un/packing routines out of toolkit.py into a new module..

— You are receiving this because your review was requested.

Reply to this email directly, view it on GitHub https://github.com/sixty-north/segpy/pull/84#event-1459797786, or mute the thread https://github.com/notifications/unsubscribe-auth/AA_uqW9gsGGRxGn5pvvypHngX1SRGGqMks5tSCj6gaJpZM4R60Z8 .

abingham commented 6 years ago

I think plug-ins is an excellent choice. We can use setuptools entry points, can’t we?

Yes, my first instinct would be to use stevedore which is all based on entry points. I'll pull something together.

abingham commented 6 years ago

I've implemented a plugin system for "Packers" using stevedore. We ship the pure-python version with segpy, and the C++ version gets installed with the extension module.

If the C++ extension is installed, we get 100% coverage of ibm_float_packer.py. But since we don't install that extension in our travis builds, we won't see that here.

We might be able to get 100% on our travis build by mocking the C++ extension; we would provide a test-only plugin that claims to be the C++ one. I think this is OK because what we're really trying to test is whether the C++ extension will be selected when it's available; we don't really care that the C++ extension is used during the tests (and in fact make no attempt to determine that a faster version of the un/packing algorithms are being used).