Closed ndevenish closed 5 years ago
Oh, also, since the reformatting one is a code-touch-singularity, I'm happy to commit it as author @luisodls so that it doesn't show blame as me for the entire codebase.
I've not looked through the voluminous diffs, but I am in favour of the bullet points as described. I have a couple of questions:
make_next
and run_all
logic paths? Is this related to the "automatic mode" of an older version of DUI?I agree that committing these changes as @luisodls makes most sense, but I imagine he will want to go through the changes in some detail in that case.
... Yes
Here's a whole load of cleanup and quality-of-life commits
Remove old versions, prototypes and tests from the main repo. These don't need to be distributed with the main program - if we want to keep them, they'd be more appropriate in a separate 'tests' branch or even prototypes repository. Keeping old backup versions can be handled by git history
Rename from
mini_idials_w_GUI
todui
. This is the name of the package and is something we should have done a while ago. Perhaps slightly controversially, I've put it in asrc/
subdirectory. This isn't universally accepted as best practice but solves several problems with the slightly-more standard layout that you can't accidentally write code that relies on the implicit PYTHONPATH of the current directory, and keeps your package namespace clearer. Since there have been problems with this in the past, this seems to fix the issue cleanlyRework the way that
main_dui
handles command arguments. It now uses argparse, making it easier to add future functionality, and ensures that--help
works. It also now sets up the python logging system, and sets default logging levels depending on a verbose parameter. I've removed the automatic/explicit command line arguments, but the actual switch code is remove in another commit.Remove
make_next
andrun_all
logic paths. These were prototypes for different behaviour. If we want to put this functionality in again in the future we probably want to do it in a different way, especially as we probably want to refactor first.Upgrade some syntax to python3-compatible. This is changing print statements to functions, and adding
__future__
imports to every file. Imports should all be relative now.Reformat entire codebase with Black This is https://github.com/ambv/black and intends to give an almost totally-PEP8-compatible formatting. It has almost no configuration, which means that there isn't temptation to bikeshed settings. It's also pretty popular nowadays. Having fully automatic formatting is now considered a pretty good idea because it means you can just write code however you want knowing that it will be reformatted afterwards. Minor potential problem - black formats python2 code but requires python3 installed somewhere to run. This shouldn't be a problem though as it's normally pretty easy to install and is something that we should be considering compatibility for anyway.
Add pre-commit hooks to install black formatting - This is a counterpart to the formatting. It's not necessary to use this, but if you install and say
pre-commit install
in the repository it will install a git hook that ensures that files are properly formatted with black on commit. Can add almost anything as a pre-commit check so this lets you ensure any style/consistency things that you want (e.g. we're working on getting libtbx-clean-clutter-style checks to ensure that you never need to see a 'clean clutter' commit on dials/cctbx again)Add configuration for flake8 - flake8 is one of the more popular code-quality-and-smell linters. This doesn't force usage, but does add configuration for the very few minor differences between it and black. So, e.g. if you run
flake8
it will give sensible output, but you won't be forced to run/adhere to it's suggestions at all.Rewrite all print() statements to use python logging - This means that the DUI output is much, much quieter. I did this transformation mostly automatically using bowler/lib2to3 with this script. Combined with the
main_dui.py
changes, only Error, Warnings and Info levels are shown by default (but there are no info-level statements at the moment), with-v
the same levels are shown but with information about source, and with-vv
and above DEBUG levels are turned on - this is the same level of output as before.Also, a couple of minor issue fixes: