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

No datablocks #2465

Merged
merged 15 commits into from
Aug 16, 2023
Merged

No datablocks #2465

merged 15 commits into from
Aug 16, 2023

Conversation

dagewa
Copy link
Member

@dagewa dagewa commented Jul 25, 2023

Two tests for dials.search_beam_position are changed to no longer use datablock.json files. After merging this and cctbx/dxtbx#570, there should be no test failures.

@codecov
Copy link

codecov bot commented Jul 25, 2023

Codecov Report

Merging #2465 (82f84db) into main (96ddc58) will increase coverage by 0.14%.
The diff coverage is 100.00%.

❗ Current head 82f84db differs from pull request most recent head 4ab6e34. Consider uploading reports for the commit 4ab6e34 to get more accurate results

@@            Coverage Diff             @@
##             main    #2465      +/-   ##
==========================================
+ Coverage   78.66%   78.80%   +0.14%     
==========================================
  Files         608      608              
  Lines       74528    74516      -12     
  Branches    10141    10140       -1     
==========================================
+ Hits        58628    58725      +97     
+ Misses      13727    13618     -109     
  Partials     2173     2173              

Copy link
Contributor

@graeme-winter graeme-winter left a comment

Choose a reason for hiding this comment

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

Change set looks good, and as a happy side-effect also removes references to more pickle as well. Thanks you. Some questions inline for my own enlightenment.

newsfragments/2465.misc Outdated Show resolved Hide resolved
tests/command_line/test_search_beam_position.py Outdated Show resolved Hide resolved
this is more consistent with elsewhere, and we _know_ that
it is pathlib.Path objects here (instead of something merely
PathLike)
@ndevenish
Copy link
Member

This looks ready. Will merge after release.

@ndevenish
Copy link
Member

Couple of remaining usages of datablock in xfail tests added by #1910 - test_phi_scan.py and test_index.py

These are probably fixed by dials/data#312, which we can probably do now.

@ndevenish
Copy link
Member

Oh, and https://github.com/dials/dials/blob/main/doc/sphinx/documentation/tutorials/3DED/lysozyme_nanocrystals.rst mentions datablocks a whole bunch, how do we feel about that tutorial?

@dagewa
Copy link
Member Author

dagewa commented Aug 16, 2023

It's kind of frozen in time, that one. It mentions specific versions of DIALS and CCP4 at the start. IIRC it was kept around following a reviewer request that the instructions for processing those datasets be made public. However, it is now quite hard to find a version of DIALS for which those instructions actually work. Nevertheless, it was already de-linked from the tutorial index page, so people are unlikely to stumble across it.

@ndevenish ndevenish enabled auto-merge (squash) August 16, 2023 09:56
@ndevenish ndevenish merged commit 8659bb6 into main Aug 16, 2023
15 of 16 checks passed
@ndevenish ndevenish deleted the no-datablocks branch August 16, 2023 19:17
toastisme pushed a commit to toastisme/dials that referenced this pull request Aug 21, 2023
The 3DED/lysozyme_nanocrystals tutorial has been left, for now, since it
is referenced in the context of a publication.

Co-authored-by: Nicholas Devenish <ndevenish@gmail.com>
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

4 participants