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

Optionally use only pycbf bindings without cbflib or cbflib_adaptbx #368

Merged
merged 33 commits into from Nov 5, 2021

Conversation

ndevenish
Copy link
Collaborator

@ndevenish ndevenish commented May 20, 2021

This is a PR for changes to optionally support using purely python-based bindings to CBFlib and libimg.

  • Split out detectorbase tests into their own test
  • Skip detectorbase tests if cbflib_adaptbx is not present (Many formats currently initialise CBF detectorbase even if they don't use, and these are tested)

IF using new pycbf:

  • Use pure python MarIP detectorbase
  • Don't compile or use cbf_read_buffered_file - use pycbf binding instead

Otherwise, everything should work exactly as it did before.

I've run this both with a plain bootstrap and also a bootstrap where I've rm -rf'd cbflib/ and cbflib_adaptbx/ before configuring the build (I don't know how cbflib_adaptbx dependency is normally pulled in, but it doesn't seem to be via a module?).

At the moment, two tests fail - both reading dials_regression/image_examples/SPring8_ADSC_SN916/Xtal17-2phi_3_015.cbf - which is the only image example in the regression tests that resolves to FormatCBFFull - #366 is a WIP example of how we could remove this dependency and read the format directly.

@codecov
Copy link

codecov bot commented May 20, 2021

Codecov Report

Merging #368 (4ac5f05) into main (6737862) will decrease coverage by 0.36%.
The diff coverage is 18.24%.

❗ Current head 4ac5f05 differs from pull request most recent head 9a5e81a. Consider uploading reports for the commit 9a5e81a to get more accurate results

@@            Coverage Diff             @@
##             main     #368      +/-   ##
==========================================
- Coverage   66.56%   66.20%   -0.37%     
==========================================
  Files         184      184              
  Lines       16652    16779     +127     
  Branches     2193     2202       +9     
==========================================
+ Hits        11084    11108      +24     
- Misses       5006     5106     +100     
- Partials      562      565       +3     

@ndevenish
Copy link
Collaborator Author

I've just merged in the branch from #366, which means that all tests now pass (excluding the detectorbase-specific tests mentioned above)

@ndevenish ndevenish changed the title Use new pycbf bindings without cbflib or cbflib_adaptbx Optionally use only pycbf bindings without cbflib or cbflib_adaptbx Nov 4, 2021
@ndevenish
Copy link
Collaborator Author

with 6737862, none of the actively tested formats in the dials_regression image examples otherwise require detectorbase to function, so the caveat is resolved.

@ndevenish ndevenish marked this pull request as ready for review November 4, 2021 15:06
@ndevenish ndevenish merged commit c28cee0 into cctbx:main Nov 5, 2021
@ndevenish ndevenish deleted the pycbf branch November 5, 2021 12:31
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.

None yet

2 participants