consolidate python linting and formatting

Problem: the developer experience is challenging because
linting and formatting are hard to debug locally, and then
the separation in the CI means 3+ clicks to see what is
going on! To fix this, this changeset adds the use of
pre-commit, which will always run linting/formatting
(and with automated fixes for a subset of the tools)
right before commit, so the developer can have some
confidence that passing locally == passing in the CI.
In the case that there is a bug in the CI, we will
now just have one place to look instead of 3+. This
should be a start of a new setup, including addition
of pre-commit and consolidation of settings into a
pyproject.toml (and setup.py for flake8) and we can
further tweak as we establish our linting preferences.

Signed-off-by: vsoch <vsoch@users.noreply.github.com>

.github/workflows/main.yml

@@ -13,56 +13,21 @@ jobs:
     - run: git fetch origin master
     - uses: flux-framework/pr-validator@master
 
-  python-format:
-    name: python format
-    runs-on: ubuntu-latest
-    steps:
-    - uses: actions/setup-python@v1
-      with:
-        python-version: 3.6
-    - uses: actions/checkout@v3
-      with:
-        ref: ${{ github.event.pull_request.head.sha }}
-        fetch-depth: 0
-    - name: install black
-      run: python3 -m pip install 'black==20.08.b1' --force-reinstall
-    - name: check format
-      run: ./scripts/check-format
-
   python-lint:
-    name: python lint
+    name: python linting
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/setup-python@v1
-      with:
-        python-version: 3.6
-    - uses: actions/checkout@v3
-      with:
-        ref: ${{ github.event.pull_request.head.sha }}
-        fetch-depth: 0
-    - name: install pylint
-      run: python3 -m pip install 'pylint==2.4.4' --force-reinstall
-    - name: run pylint
-      run: ./scripts/pylint
-
-  mypy:
-    name: mypy
-    runs-on: ubuntu-latest
-    steps:
-    - uses: actions/setup-python@v1
-      with:
-        python-version: 3.6
     - uses: actions/checkout@v3
       with:
         ref: ${{ github.event.pull_request.head.sha }}
         fetch-depth: 0
-    - name: install pylint
-      run: python3 -m pip install 'mypy==0.770' --force-reinstall
-    - name: run mypy
-      run: mypy
+    - name: install linting and formatting deps
+      run: pip install -r scripts/requirements-dev.txt
+    - name: format and linting checks
+      run: pre-commit run --all-files
 
   check-sched:
-    needs: [ python-format, python-lint, mypy ]
+    needs: [python-lint]
     name: flux-sched check
     runs-on: ubuntu-latest
     steps:
@@ -81,7 +46,7 @@ jobs:
         src/test/docker/docker-run-checks.sh -j 4 -i bionic
 
   check-accounting:
-    needs: [ python-format, python-lint, mypy ]
+    needs: [python-lint]
     name: flux-accounting check
     runs-on: ubuntu-latest
     steps:

.gitignore

@@ -36,6 +36,8 @@ libltdl/
 # docs intermediate files
 /doc/man*/*.xml
 /doc/_build
+/doc/man
+etc/completions/flux
 
 # Object files
 *.o
@@ -99,6 +101,10 @@ compile_flags.txt
 .idea
 .clangd
 
+# local environmnt
+env
+
 # Rules to ignore auto-generated test harness scripts
 test_*.t
 !/src/bindings/python/test_commands/test_runner.t
+src/bindings/python/_flux/*.h

.pre-commit-config.yaml

@@ -0,0 +1,38 @@
+files: src/bindings/python/flux
+repos:
+  - repo: https://github.com/pre-commit/pre-commit-hooks
+    rev: v4.2.0
+    hooks:
+      - id: check-added-large-files
+      - id: check-case-conflict
+      - id: check-docstring-first
+      - id: check-shebang-scripts-are-executable
+      - id: end-of-file-fixer
+      - id: trailing-whitespace
+      - id: mixed-line-ending
+
+  - repo: local
+    hooks:
+      - id: black
+        name: black
+        language: python
+        types: [python]
+        entry: black
+
+      - id: isort
+        name: isort
+        language: python
+        types: [python]
+        entry: isort
+
+      - id: flake8
+        name: flake8
+        language: python
+        types: [python]
+        entry: flake8
+
+      - id: mypy
+        name: mypy
+        language: python
+        types: [python]
+        entry: mypy

doc/python/developer.rst

@@ -0,0 +1,102 @@
+Flux Python Development
+=======================
+
+These are notes for Python developers of Flux!
+
+
+Linting
+-------
+
+We have added support for basic automation of linting tasks, which you are
+free to use or not. If you choose to not use these tools locally, they
+will be checked in the continuous integration (and you can make changes 
+accordingly).
+
+.. note:: The ``flux`` package which is used to interact with Flux *cannot* yet be installed into a virtual environment with pip or conda.
+
+To install the requirements, we recommend creating a virtual environment,
+and then installing:
+
+.. code-block:: console
+
+    $ python -m venv env
+    $ source env/bin/activate
+
+Then install the dependencies for linting:
+
+.. code-block:: console
+
+    $ pip install -r scripts/requirements-dev.txt
+
+This includes linting tools black, isort, mypy, and flake8, and pre-commit
+to automate running them. You can then install the hook:
+
+.. code-block:: console
+
+    $ pre-commit install --hook-type pre-commit
+
+You can optionally run pre-commit at any time like so:
+
+.. code-block:: console
+
+    $ pre-commit run --all-files
+    
+You'll see (on the first run) a bunch of errors, but likely the tools will
+fix them for you, and a new run will be much cleaner, with only a few manual
+fixes necessary:
+
+.. code-block:: console
+
+    check json...........................................(no files to check)Skipped
+    check yaml...........................................(no files to check)Skipped
+    fix end of files.........................................................Passed
+    trim trailing whitespace.................................................Passed
+    mixed line ending........................................................Passed
+    black....................................................................Passed
+    isort....................................................................Passed
+    flake8...................................................................Passed
+    mypy.....................................................................Passed
+
+
+And that's it! The check run in the CI is equivalent to here, so you should
+always be able to reproduce failures. And if you install the pre-commit hook,
+you can largely prevent them by always catching them before commit.
+
+Development Environment
+-----------------------
+
+To easily develop with Python, you can use a prebuilt container. Build as follows
+from the root of the Flux repository:
+
+.. code-block:: console
+
+    $ docker build -t ghcr.io/flux-framework/flux-ubuntu -f etc/docker/ubuntu/Dockerfile .
+
+And then shell inside, either without binding your code:
+
+.. code-block:: console
+
+    $ docker run -it ghcr.io/flux-framework/flux-ubuntu
+
+or with binding your code (for interactive development):
+
+.. code-block:: console
+
+    $ docker run -v $PWD:/code -it ghcr.io/flux-framework/flux-ubuntu
+
+In which case you should build things once more:
+
+.. code-block:: console
+
+    $ ./autogen.sh && make && make install
+        
+and then you can add the site-packages to your PYTHONPATH, make changes on your
+local machine, and run ``make install`` to install and then test (this will move
+the Python files from your local directory into the final install destination).
+
+.. code-block:: console
+
+    $ export PYTHONPATH=/usr/local/lib/flux/python3.10:/usr/local/lib/python3.10/site-packages
+
+
+Happy Developing!

doc/python/index.rst

@@ -13,7 +13,7 @@ command line are possible in Python as well via the ``flux`` package.
    basics
    job_submission
    complete_api
-
+   developer
 
 Indices and tables
 ------------------

etc/docker/ubuntu/Dockerfile

@@ -47,7 +47,7 @@ COPY . /code
 
 RUN chmod +x etc/gen-cmdhelp.py && \
     ./autogen.sh && \
-    ./configure --prefix=/usr/local --without-python && \
+    ./configure --prefix=/usr/local && \
     make && \
     make install && \
     ldconfig

pyproject.toml

@@ -0,0 +1,52 @@
+[tool.black]
+line-length = 100
+target-version = ['py37']
+
+[tool.isort]
+profile = "black"
+line_length = 100
+
+[tool.mypy]
+python_version = 3.7
+files = ["src/cmd/**/*.py", "src/bindings/python/flux", "t/python/*.py"]
+mypy_path = ["src/bindings/python", "t/python/tap", "t/python"]
+allow_redefinition = true
+exclude = ["src/bindings/python/flux/utils/parsedatetime/"]
+
+# Having the cache makes spurious errors about looking up handle
+# It's slightly slower, but more correct to not have it
+cache_dir="/dev/null"
+namespace_packages = true
+ignore_errors = true
+ignore_missing_imports = true
+
+  [[tool.mypy.overrides]]
+  module = "subflux"
+  ignore_errors = true
+  ignore_missing_imports = true
+  follow_imports = 'skip'
+
+  [[tool.mypy.overrides]]
+  module = [
+    "_flux._core",
+    "_flux._security",
+    "_flux._hostlist",
+    "_flux._idset",
+    "_flux._rlist",
+    "_flux.cffi",
+    "setuptools",
+    "jsonschema",
+    "flux.utils",
+    "flux.utils.parsedatetime",
+    "flux.utils.tomli",
+    "tomllib",
+    "pycotap",
+
+    # These are temporary while we find a way to generate stubs for flux.constants
+    "flux.job.list",
+    "flux.core.handle",
+    "python.t1000-service-add-remove"
+  ]
+  ignore_errors = true
+  ignore_missing_imports = true
+  follow_imports = 'skip'