Modernize project: Python 3.11, pyproject.toml, ruff, uv, drop reques… - #115
Merged
Conversation
…ts_html Brings the project onto current Python tooling and packaging conventions so new contributors can install and work with it without fighting deprecated tools. Existing config was inconsistent (Makefile pinned 3.8, setup.py said 3.10, CI ran 3.8) and the legacy black/isort/flake8/pylint/pydocstyle stack plus an unmaintained requests_html dep made the install path brittle. Packaging - New pyproject.toml (PEP 621) replaces setup.py + setup.cfg + requirements.txt + pytest.ini. Dynamic version from ml4floods/__init__.py. Optional extras: dev / tests / docs. - Generated uv.lock for reproducible installs (232 packages). Tooling - Ruff replaces black + isort + flake8 + pylint + pydocstyle. Conservative initial rule set (E/F/W/I); UP/B and a number of legacy issue codes are ignored for now and intended to be enabled per-module in follow-ups. - mypy + pytest config moved into pyproject.toml. - Makefile rewritten for uv + ruff (install-dev, lint, format, type, test, check, build, publish). Conda flow removed — environment.yml never existed. - New .pre-commit-config.yaml: ruff, ruff-format, plus basic hygiene hooks. CI - deploy.yml: trigger fixed master -> main, Python 3.8 -> 3.11, uv install, publish_dir corrected (docs/_build/html -> jupyterbook/_build/html), action versions bumped (checkout v2 -> v4, setup-python v1 -> setup-uv v3, actions-gh-pages v3.6.1 -> v4). - New test.yml with three jobs: ruff lint+format check, mypy (continue-on-error to surface but not gate), pytest. Source - Drop requests_html (unmaintained since 2020). Replaced HTMLSession + r.html.find / .html.links with requests.Session + BeautifulSoup in copernicusEMS/activations.py and unosat/unosat_download.py. Removed dead HTMLSession import from serve/tileserver/helpers.py. - Auto-formatter pass touched ~80 files (whitespace, EOF newlines, quote style, isort). No behavior changes. Misc - .idea/ untracked and added to .gitignore. .venv/ added to .gitignore. Known follow-ups (out of scope here) - 150 ruff lint issues remain in legacy code (F821 undefined names, bugbear B006/B007/B008/B028, pyupgrade UP008/UP031, etc.) — currently ignored. Address per module. - Three test files reference symbols that don't exist on main (WorldFloodsImage, create_folder); collection errors are pre-existing, not caused by this work. - 24 .ipynb notebooks under notebooks/ and jupyterbook/ still mention requests_html in stale cell output; not in scope.
These test modules were testing APIs (create_folder, get_files_in_directory, WorldFloodsImage, data_download) that were intentionally removed in PR #97 (Tutorial Updates, ~1 year ago). They have not been runnable since, and the symbols they import don't exist in the package or anywhere else in the repo. Also drops two zero-byte placeholder test files. After this, `pytest tests` collects and passes cleanly (5 tests). The CI test job added in the modernization PR is now actually green. Removed: - tests/data/worldfloods/test_dataset.py - tests/data/worldfloods/test_download.py - tests/data/worldfloods/test_loader.py (empty) - tests/data/worldfloods/test_prepare_data.py (empty) - tests/preprocess/test_tiling.py
Resolves all 16 F821 undefined-name lint errors across the package and scripts (13 in ml4floods/, 3 in scripts/) so the rule can be removed from ruff's ignore list. F821 is now enforced going forward. Three classes of issue, all latent for ~1 year: unosat_download.py — `os` was used (os.path.basename, os.listdir, os.mkdir, os.path.join, etc.) but never imported. The download_shapefiles code path would have NameError'd on first call. Added the missing `import os`. indexer.py:144 — typo. The CLI flag is `args.log_level` (used on line 142) but the error message tried to interpolate an undefined `loglevel`. Switched to f-string with args.log_level. create_worldfloods_dataset.py — `worldfloods_old_gcp_paths` was dead code referencing the GCPPath class deleted in PR #97 (Tutorial Updates). The function used GCPPath, .check_if_file_exists, .full_path, .file_name on plain strings and would have crashed on any call. Deleted it. The `generate_item` function had it as the default value for `paths_function`; switched the default to the working `worldfloods_extra_gcp_paths` (same 5-tuple signature). All real callers in scripts/ already pass paths_function explicitly. generate_ground_truth_wf.py — `main_worldlfoods_original` was the same dead code at script level (also using GCPPath). Reachable only via `--dataset ""`, which produced a NameError immediately. Removed the function, removed the empty-string choice from argparse, and collapsed the now-redundant branching in main(). Verification: ruff check (with F821 enabled), pytest, and pre-commit all pass.
Removes 4 unused imports and 6 unused local assignments so F401 and F841 can be removed from ruff's ignore list and enforced going forward. The cleanups: ee_download.py — drop `cirrus_bit_mask` (used only by code that's been commented out) and the dead comment that referenced it. Drop unused `pol` and `area_of_interest_geojson` assignments in two functions where neither the value nor the side effect of `mapping()` was needed. unosat_download.py — `get_flood_shapefiles` was issuing one HTTP GET per map link and discarding the response, then calling `get_flood_shape_and_meta` which fetches the same page itself. Removing the wasted request halves traffic in this loop. tiling.py — `sub_image = dataset.read(indexes=bands, window=...)` was read into a local that was never used; the actual write a few lines down calls `dataset.read(window=..., boundless=True, fill_value=0)` (no band selection). Note: the surrounding code has a separate latent issue — `window_meta["channels"] = bands` claims fewer bands than the write actually produces — but that's outside the scope of an F841 cleanup. unused imports — drop `requests` from serve/__init__.py (the only references are in commented-out lines), drop `pathlib.Path`, `write_json_to_gcp`, and `worldfloods_output_files` from the worldfloods GT script (orphaned by the prior dead-code removal), and drop unused `fs` in download_images_inference.py. After this, ruff's only remaining ignored rules are formatter-only (E501) and judgment-call style codes (E402, E711, E721, E722, E731, E741, F403, F405). Pyflakes-correctness checks (F-series) are now all on except for star-import ones which need an evaluation of the actual re-export contracts in __init__.py files.
Resolves all 29 UP violations and adds the UP rule family to ruff's
select set so future regressions get caught.
UP008 (13 sites, auto-fixed) — `super(ClassName, self).method()` is the
Python 2 form. Modern Python uses bare `super().method()`.
UP031 / UP032 (16 sites, mostly auto-fixed) — converted printf-style `%`
formatting and `.format()` calls to f-strings. The 7 sites ruff couldn't
auto-fix (multi-line `%`, multi-arg tuples, nested format specs) were
done by hand. Examples:
- `"%.1f%%\n%d/%d" % (p, c, s)` → `f"{p:.1f}%\n{c}/{s}"`
- `"Date pre event {} is after date post event {}".format(d1, d2)` →
f-string with nested `.strftime()` calls
No behavior change in any case — the formatted output is byte-equivalent.
Tests, ruff, and pre-commit all pass. The `B` (bugbear) ruleset is the
last opt-in tier left to enable; ~50 sites remain there but most are
judgment calls (mutable defaults, raise-without-from, etc.) that need
per-site review rather than mechanical fixes.
Mechanical fix of 11 E711 violations across three files. Identity comparison is the correct way to test for None — `==` only happens to work when no `__eq__` is defined to override it. Behavior is identical for all current call sites, but `is`/`is not` is unambiguous and ~3x faster (no method dispatch). Enables E711 in ruff so any future regression is caught at lint time.
Resolves the remaining mid-tier lint debt across six rules and removes each from ruff's ignore list, leaving only E501 (formatter-managed) and E731 (lambda assignment, judgment call) ignored. E402 (24 sites) — Module-level imports placed below `logging.basicConfig` calls in three serve modules (REST_mosaic, ingest, modelserver/app), and one orphaned import buried mid-file in viewer/serve.py. The basicConfig-then-imports pattern is unintentional; moving the basicConfig call below the imports is the standard order and equivalent in behavior. The viewer import was merged into the existing `from shapely.geometry import shape` at the top. E721 (1 site) — `type(x) == gpd.GeoDataFrame` → `isinstance(x, gpd.GeoDataFrame)`. Subclasses now match correctly; no current call site has a subclass but this is the standard idiom. E722 (3 sites) — Bare `except:` clauses replaced with `except Exception:` (or `except FileNotFoundError:` where the failure mode is specific). Bare except catches BaseException including KeyboardInterrupt and SystemExit, which is almost never intended. The ingest.py site additionally chains the original via `raise ValueError(...) from e` to preserve the underlying cause. E741 (3 sites) — Renamed loop variable `l` → `label` in three identical metric helpers (calculate_iou/recall/precision). `l` is hard to distinguish from `1` and `I` in many fonts. F403 + F405 (1 + 2 sites) — `helpers.py` had `from ml4floods.data.copernicusEMS.activations import *` solely to re-import `geopandas as gpd` indirectly. No other symbols from `activations` were actually used in the file. Replaced the star import with a direct `import geopandas as gpd`. modelserver/app.py also had a `from ml4floods.models.model_setup import get_model` planted between two top-level statements; merged it into the existing import line above.
Six lambdas-bound-to-names in model_setup.py converted to proper `def` functions. Each lambda would have shown up as `<lambda>` in stack traces and lacked a meaningful `__name__` for debugger / decorator introspection. Behavior is unchanged — same single-expression bodies, same closures over `model`, `device`, `activation_fun`. In `get_pred_function` the `pred_fun = lambda ti: activation_fun(model(...))` appeared on both branches of an if/else (with the if branch wrapping it in `padded_predict(...)`). Hoisted the def out of the conditional and let `padded_predict` rebind `pred_fun` only when `module_shape > 1`, so the function isn't redefined unnecessarily. E731 is now enforced. Only E501 remains in ruff's ignore list, and that one is delegated to ruff-format. Every other rule in `select` (E, F, W, I, UP) is fully on.
Smoke-tested the package on torch 2.11 / numpy 2.4.4 / pytorch-lightning 2.6.1 / Python 3.12. Surfaces and fixes the real API-compat issues that were lurking on this branch. Real bugs fixed: metrics.py:420 — `thresholds_water: np.array | None = None`. `np.array` is the array-constructor function, not a type, so PEP 604's `X | None` union evaluation hit `TypeError: unsupported operand type(s) for |` *at module import time*. Cascaded to break four downstream modules (metrics, model_setup, worldfloods_model, plot_utils). Replaced with `np.ndarray`. ml4floods/data/worldfloods/download.py — Removed entirely. Imported `download_data_from_bucket` and `save_file_from_bucket` from ml4floods.data.utils, both of which were deleted in PR #97 a year ago. Zero callers remained anywhere in the codebase or notebooks. Missing core dependencies — `pyproject.toml` had been pinning the package as installable but several of its modules import packages that were never declared: scikit-image used by create_gt, postprocess, scripts/inference, serve/tileserver/ingest fs (pyfilesystem2) used by data/index/indexer and data/unosat/unosat_download geojson used by serve/tileserver/{app,helpers} lxml used transitively via pandas.read_html in copernicusEMS/activations.table_floods_ems Added all four to `dependencies`. Optional extras introduced: serve = [flask, gunicorn] — pulls in the Flask web servers cloudmasks = [segmentation-models-pytorch, s2cloudless] — opt-in alternative cloud masks kappazeta = [tensorflow] — separate because of TF's size Lightning + ML stack smoke test: 4/4 architectures (UNet, UNet_dropout, SimpleCNN, SimpleLinear) round-trip a (2,13,128,128) tensor; WorldFloodsModel runs forward + training_step + backward and produces non-zero gradients. Remaining import failures (3 of 66 modules) are *environment-dependent*, not API breakage: serve/tileserver/app.py and serve/modelserver/app.py both make GCS fetches at module-import time (table_floods_ems(), get_default_config). They require GOOGLE_APPLICATION_CREDENTIALS + an internet route to emergency.copernicus.eu and gs://ml4cc_data_lake. This was true on main as well. data/cmkappazeta needs tensorflow (intentionally excluded from default install; available via `pip install ml4floods[kappazeta]`).
The existing `pypi/pyversions/ml4floods` badge is dynamic — it reads the classifiers/`requires-python` from whatever's currently on PyPI, so it won't reflect the new 3.11 floor until a release is cut after this work lands. Adding a static badge alongside it gives readers an immediate, unambiguous signal of the support floor declared in pyproject.toml.
gonzmg88
approved these changes
May 6, 2026
Cleanup: tests, lint debt, Python 3.11 compat, missing deps
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…ts_html
Brings the project onto current Python tooling and packaging conventions so new contributors can install and work with it without fighting deprecated tools. Existing config was inconsistent (Makefile pinned 3.8, setup.py said 3.10, CI ran 3.8) and the legacy black/isort/flake8/pylint/pydocstyle stack plus an unmaintained requests_html dep made the install path brittle.
Packaging
Tooling
CI
Source
Misc
Known follow-ups (out of scope here)