xia2 / screen19

Screening program for small-molecule single-crystal X-ray diffraction data
https://pypi.org/project/screen19/
BSD 3-Clause "New" or "Revised" License
2 stars 3 forks source link

Update import sorting behaviour #33

Closed benjaminhwilliams closed 4 years ago

benjaminhwilliams commented 4 years ago

Change of plan. Leave the updating of pre-commit hooks to the new GitHub workflow (see #36).

Update isort behaviour

• Create separate groups for CCTBX, DXTBX, DIALS & xia2. • Use the new profile for compatibility with Black. • Float all imports with isort.

Since isort version 5, the default behaviour is not to float imports unless explicitly configured to do so. See https://pycqa.github.io/isort/docs/major_releases/introducing_isort_5/#sort-imports-anywhere and https://pycqa.github.io/isort/docs/configuration/options/#float-to-top

Note that indented imports are not floated.

ndevenish commented 4 years ago

I'd probably be tempted to jumble dxtbx, dials and xia2 together - there isn't that many uses of them (and in fact, it looks like xia2 isn't used at all?)

Weirdly, the float_to_top appears to float the imports above the docstring in __init__.py - here is the run applied - which is probably not what you want. I understand the potential use of the setting, letting you declare inline whilst developing, but it's probably better to have something that you know about that can use manually.

FWIW Aside: My generic float-everything (top level and inside functions) script at https://github.com/ndevenish/tbxtools/blob/master/fixers/fix_float_imports.py can be useful for this sort of thing (requires packages bowler, isort to run) and it only finds things it doesn't want to move at the moment, so I'm not convinced the potential breakage is worth it:

Not floating ./screen19/minimum_exposure.py:242 as matplotlib
Not floating ./screen19/minimum_exposure.py:245 as matplotlib
Not floating ./screen19/screen.py:259 as matplotlib
Not floating ./screen19/screen.py:262 as matplotlib
Not floating ./tests/conftest.py:6 (dials_data as _) as has comments
benjaminhwilliams commented 4 years ago

Weirdly, the float_to_top appears to float the imports above the docstring in __init__.py

Ooh er! Good spot.

I'd probably be tempted to jumble dxtbx, dials and xia2 together - there isn't that many uses of them (and in fact, it looks like xia2 isn't used at all?)

xia2 isn't used but it's conceivable that it might be in the future. But I think you're right, I'll lump them altogether into one block.