Remove datablock

[graeme-winter]

Repeat of #504

[codecov]

Codecov Report

Merging #570 (70fe56d) into main (5e079c7) will decrease coverage by 1.14%.
Report is 7 commits behind head on main.
The diff coverage is 6.92%.

❗ Current head 70fe56d differs from pull request most recent head bc385e9. Consider uploading reports for the commit bc385e9 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #570      +/-   ##
==========================================
- Coverage   39.10%   37.96%   -1.14%     
==========================================
  Files         181      182       +1     
  Lines       15864    15817      -47     
  Branches     3066     3037      -29     
==========================================
- Hits         6203     6005     -198     
- Misses       9075     9244     +169     
+ Partials      586      568      -18     

[graeme-winter]

N.B.

Results (54.41s):
     748 passed
       5 failed
         - tests/test_datablock.py:90 test_json2
         - tests/test_datablock.py:80 test_json
         - tests/test_datablock.py:70 test_pickling
         - tests/test_datablock.py:110 test_with_bad_external_lookup
         - tests/test_datablock.py:133 test_with_external_lookup
       7 xfailed
       2 skipped

were previously missed

[graeme-winter]

In fairness, removing test_datablock alongside removal of datablock seems like a trivial and obvious way to resolve this.

[graeme-winter]

Found and expired offending regression tests. They were not useful if we do not have datablock. If there is useful test coverage in there should be added as a new PR using experiments.

[graeme-winter]

OK, waded in this time around and 99% expunged the datablock from history -> there are instances of the word datablock but now all the regression tests pass, the regular ones pass, and so I cam calling this bad boy done.

Results (84.99s):
    1113 passed
       7 xfailed
       2 skipped

[graeme-winter]

OK, now I have a more reasonable assessment of the test status in dials using dials_regression - it is not so bad but there are fixes needed which could be quite annoying....

=========================== short test summary info ============================
SKIPPED [1] tests/command_line/test_powder_calibrate.py:7: could not import 'pyFAI': No module named 'pyFAI'
SKIPPED [1] tests/command_line/test_stills_process.py:125: could not import 'mpi4py': No module named 'mpi4py'
xfail tests/algorithms/indexing/test_phi_scan.py::test_run
xfail tests/algorithms/refinement/test_scan_varying_prediction_parameters.py::test_SparseFlex_select_intersection[True]
xfail tests/algorithms/indexing/test_index.py::test_refinement_failure_on_max_lattices_a15
XPASS tests/util/test_masking.py::test_shadow_plot Failing due to deprecation warning in output

Results (251.08s):
    1342 passed
       1 xpassed
       6 failed
         - tests/algorithms/refinement/test_hierarchical_detector_refinement.py:62 test1
         - tests/algorithms/refinement/test_two_theta_refinement.py:192 test_refinement
         - tests/command_line/test_search_beam_position.py:143 test_search_single
         - tests/command_line/test_search_beam_position.py:210 test_multi_sweep_fixed_rotation
         - tests/command_line/test_search_beam_position.py:56 test_search_multiple
         - tests/command_line/test_refine_error_model.py:6 test_standalone_error_model_refinement_on_scaled_data
       3 xfailed
       1 skipped

[dagewa]

I'm planning to move everything that gets touched by tests from $DIALS_REGRESSION/refinement_test_data/ to somewhere in dials_data, but I haven't had time to do that yet.

[dagewa]

I started moving files, see dials/data-files#46. Once they're available in dials_data I'll update any remaining datablocks to be experiments

[dagewa]

dials/dials#2346 removes datablocks from test_hierarchical_detector_refinement.py and test_two_theta_refinement.py from the list above. In test_search_beam_position.py it also fixes test_search_multiple, but I was not sure about changes to test_search_single and test_multi_sweep_fixed_rotation. Probably they could be rewritten to use dials-data instead. The remaining failure, in test_refine_error_model.py seems to have nothing to do with datablocks.

[dagewa]

To confirm, after merging dials/dials#2346, there are now two failures in DIALS tests, using dxtbx from this branch, both in test_search_beam_position.py:

[gw3] linux -- Python 3.10.8 /home/fcx32934/sw/cctbx/build/../conda_base/bin/python
/home/fcx32934/sw/cctbx/modules/dials/tests/command_line/test_search_beam_position.py:159: in test_search_single
    search_beam_position.run([experiments_path, pickle_path])
/home/fcx32934/sw/cctbx/conda_base/lib/python3.10/contextlib.py:79: in inner
    return func(*args, **kwds)
/home/fcx32934/sw/cctbx/modules/dials/src/dials/command_line/search_beam_position.py:477: in run
    params, options = parser.parse_args(args, show_diff_phil=False)
/home/fcx32934/sw/cctbx/modules/dials/src/dials/util/options.py:902: in parse_args
    raise Sorry(msg)
E   Sorry: Unable to handle the following arguments:
E     /home/fcx32934/sw/cctbx/modules/dials_regression/indexing_test_data/phi_scan/datablock.json did not appear to conform to any expected format:
E       - ExperimentList: Expected dictionary, not <class 'list'>
E       - Reflections:    Appears to be an invalid pickle file
________________________________________________________ test_multi_sweep_fixed_rotation ________________________________________________________
[gw3] linux -- Python 3.10.8 /home/fcx32934/sw/cctbx/build/../conda_base/bin/python
/home/fcx32934/sw/cctbx/modules/dials/tests/command_line/test_search_beam_position.py:221: in test_multi_sweep_fixed_rotation
    search_beam_position.run(reflection_files + experiment_files)
/home/fcx32934/sw/cctbx/conda_base/lib/python3.10/contextlib.py:79: in inner
    return func(*args, **kwds)
/home/fcx32934/sw/cctbx/modules/dials/src/dials/command_line/search_beam_position.py:477: in run
    params, options = parser.parse_args(args, show_diff_phil=False)
/home/fcx32934/sw/cctbx/modules/dials/src/dials/util/options.py:902: in parse_args
    raise Sorry(msg)
E   Sorry: Unable to handle the following arguments:
E     /home/fcx32934/sw/cctbx/modules/dials_regression/indexing_test_data/multi_sweep/SWEEP1/index/9_datablock_import.json did not appear to conform to any expected format:
E       - ExperimentList: Expected dictionary, not <class 'list'>
E       - Reflections:    Appears to be an invalid pickle file
E     /home/fcx32934/sw/cctbx/modules/dials_regression/indexing_test_data/multi_sweep/SWEEP2/index/20_datablock_import.json did not appear to conform to any expected format:
E       - ExperimentList: Expected dictionary, not <class 'list'>
E       - Reflections:    Appears to be an invalid pickle file

[dagewa]

Just had a thought, the very quickest way through here would probably be to run something like this on the remaining datablocks:

from dxtbx.datablock import DataBlockFactory
from dxtbx.model.experiment_list import ExperimentListFactory
db = DataBlockFactory.from_json_file("datablock.json", check_format=False)[0]
el = ExperimentListFactory.from_datablock_and_crystal(db, crystal=None)
el.as_file("foo.expt")

I could do that and push the results back to dials_regression, to sit alongside the currently used datablock files. Would that be a problem though? Some people use an svn export rather than checkout of dials_regression, so they'd have to update?

I know moving the remaining tests to dials-data would be nicer, but I won't have time to do that myself.

[phyy-nx]

Sounds good to me

[ndevenish]

I could do that and push the results back to dials_regression, to sit alongside the currently used datablock files. Would that be a problem though?

Please, do not do this. We don't update dials_regression, and haven't done since Jan 2020. We don't track installations that do or do not update from SVN. dials_regression should be read-only, as it has for several years (there have been a couple of inadvertant commits that should have all been reverted).

Any new files should be exclusively added to dials-data.

[dagewa]

Sorry, I don't have time to work on this any more. I moved all refinement tests to dials-data recently and seeing as there is generally a lack of movement in other files getting moved to dials-data I thought adding experiments to dials_regression was an easy solution to get this across the line. I leave it to others to fix their tests then.

[dagewa]

Confirmed locally that once dials/dials#2465 is merged, then this can be merged too with no test failures.