Move dxtbx to src/ layout

[ndevenish]

An embedded layout (e.g. at least in a subfolder to the root) is a requirement for moving dxtbx towards being a relatively normal installable package. src/ layout provides isolation against namespace contamination with the other folders in the package root - both for headers, and PYTHONPATH purposes.

• Updated SConscript to retain identical build behaviour and provide include paths to the new location
• Created setup.py to (editable or otherwise) install the dxtbx library into an existing environment. It generates the list of console_scripts entry points and Format registry entry points automatically. I've tried to keep the actual setup.py somewhat plain, and moved the dynamic logic into a build.py file
• Modified libtbx_refresh.py to editable-install the dxtbx package when configured as part of a libtbx environment

A possible... complication, is that it appears that the general "generate console_scripts entrypoints as libtbx dispatchers" behaviour appears to be somewhat related to calling libtbx.pkg_utils.define_entry_points. At least, the dispatchers for things like bin/pytest have disappeared on my environments. Not sure how to work around that?

[Anthchirp]

This will need some rigorous testing. One to discuss next Thursday, I assume.

A possible... complication, is that it appears that the general "generate console_scripts entrypoints as libtbx dispatchers" behaviour appears to be somewhat related to calling libtbx.pkg_utils.define_entry_points. At least, the dispatchers for things like bin/pytest have disappeared on my environments. Not sure how to work around that?

No, libtbx is fine as it is.

The bin/pytest dispatcher disappeared because it was generated via dxtbx (in command_line/pytest_launcher.py). In libtbx *tbx packages are evaluated differently from python packages: Any libtbx command_line/ script will only get an 'outer' dispatcher generated in /build/bin, whereas a python package console_scripts entry point will get the regular 'inner' python dispatcher underneath /base (not controlled or affected by libtbx), and nothing in /build.
The problem is then that anything launched from a regular python dispatcher will not be able to import anything from underneath the modules/ directory (unless installed as python package with pip -e), and certainly not any external modules from the /build directory - which meant that the regular pytest dispatcher was unable to test eg. dxtbx.

By transforming dxtbx into a standard python module you implicitly removed the build/bin dispatchers, which I guess is also the cause for the build errors you can see on Azure.

So how can you fix this? It should be relatively easy. Tell libtbx to generate an additional outer dispatcher for the inner dispatcher. Set entry points libtbx.dispatcher.script with OUTER=INNER where OUTER is the name of the outer dispatcher and INNER the name of the inner dispatcher.
Once you know what to look for you'll find this pattern all over the place in more or less all our packages:

• fastdp
• dlstbx
• ispyb-api
• zocalo
• xia2
• ...

While you're there - could you please also add an entry point libtbx.precommit: "dxtbx=dxtbx"? This will ensure pre-commits will installed automatically on the repository if it sits underneath modules/. Purely for the benefit of those of us who don't have it set up globally.

[ndevenish]

I suspect most of these errors now are due to #383

[codecov]

Codecov Report

Merging #382 (eaf06f5) into main (fa9f778) will increase coverage by 14.60%.
The diff coverage is 5.63%.

❗ Current head eaf06f5 differs from pull request most recent head 724d74d. Consider uploading reports for the commit 724d74d to get more accurate results

@@             Coverage Diff             @@
##             main     #382       +/-   ##
===========================================
+ Coverage   49.58%   64.18%   +14.60%     
===========================================
  Files         235      175       -60     
  Lines       19489    15043     -4446     
  Branches     2761     2051      -710     
===========================================
- Hits         9663     9656        -7     
+ Misses       9286     4846     -4440     
- Partials      540      541        +1     

[ndevenish]

So, I've put it back in, but only if it is already installed (or it can't tell). I think that's possibly cleaner than continuing to install it into new environments.

[ndevenish]

CCTBX bootstrap for --builder=xfel and --builder=phenix both build fine (to a point - the phenix builder didn't download phenix or boost, so either broken or not the normal way).

Running libtbx.refresh with the new changes (normal dials bootstrap) seem to run into:

Traceback (most recent call last):
  File "/tmp/mep23677/dist_dials/modules/cctbx_project/libtbx/env_config.py", line 3200, in <module>
    unpickle().refresh()
  File "/tmp/mep23677/dist_dials/modules/cctbx_project/libtbx/env_config.py", line 2249, in refresh
    self.generate_entry_point_dispatchers()
  File "/tmp/mep23677/dist_dials/modules/cctbx_project/libtbx/env_config.py", line 2123, in generate_entry_point_dispatchers
    self.write_dispatcher(
  File "/tmp/mep23677/dist_dials/modules/cctbx_project/libtbx/env_config.py", line 1640, in write_dispatcher
    raise RuntimeError("Multiple sources for dispatcher:\n"
RuntimeError: Multiple sources for dispatcher:
  target file:
    "/tmp/mep23677/dist_dials/build/bin/cxi.image2pickle"
  source files:
    "/tmp/mep23677/dist_dials/modules/dxtbx/src/dxtbx/command_line/image2pickle.py"
   =relocatable_path(anchor="/tmp/mep23677/dist_dials/build", relocatable="../modules/dxtbx/src/dxtbx/command_line/image2pickle.py")
    "/tmp/mep23677/dist_dials/conda_base/bin/cxi.image2pickle"
   =relocatable_path(anchor="/tmp/mep23677/dist_dials/build", relocatable="../conda_base/bin/cxi.image2pickle")

[Anthchirp]

So, I've put it back in, but only if it is already installed (or it can't tell). I think that's possibly cleaner than continuing to install it into new environments.

More effort than what I'd have put in for something that will be removed anyway 🤷

The error is not that surprising - for some reason you explicitly tell libtbx to generate dispatchers for the src/dxtbx/command_line directory in libtbx_config. Those will of course conflict with the ones you tell it to generate via the libtbx.dispatcher.script entry point.

To be honest, I would suggest completely eliminating the command_line directory, rather than keeping it around and half-depending on libtbx-like mechanisms. That is the approach we took when we implemented DC1 for xia2 (xia2/xia2#528) and I think that was the correct choice.

[Anthchirp]

Haven't tested the upgrade paths, but bootstrapping looks good to me

[ndevenish]

I'm just trying on a very old distribution on my old mac. It's rather slow to rebuild everything though...

[ndevenish]

This worked fine. It did need three libtbx.refresh for an existing (old) installation. This appears to somewhat be similar to the pattern:

1. No dispatchers are removed, the metapackage is updated to be empty, and dxtbx package installed. The registry doesn't work.
2. The dispatchers are removed, since the metapackage has been removed.
3. The new dispatchers are installed.

I think this comes from the limitation that a packages dispatchers are handled before we can fiddle with entry_points in libtbx_refresh. It's a bit annoying, but leaves us in a better place, and new(er?) installs are unaffected.

[ndevenish]

Okay, so the plan was to merge this, and patch the new dxtbx include path into the dials include path temporarily. Except that won't work because xfel recalculates the dials path: https://github.com/cctbx/cctbx_project/blob/8a572444f4c6e6787c6fd4af2fa1569adc331d9b/xfel/SConscript#L7-L8.

So I'm going to leave this here until cctbx-base 2021.6 is release, which should hopefully be around beginning of... july