Skip to content

Create Python package - #19

Open
Jean-Guillaume Durand (jeangud) wants to merge 4 commits into
microsoft:mainfrom
jeangud:packaging
Open

Jean-Guillaume Durand (jeangud) wants to merge 4 commits into
microsoft:mainfrom
jeangud:packaging

Conversation

@jeangud

@jeangud Jean-Guillaume Durand (jeangud) commented Aug 12, 2024 •

Copy link
Copy Markdown
Contributor

Description

SatCLIP currently has to be used from a clone with satclip/ on PYTHONPATH, and it doesn't declare its dependencies (#8). This PR turns it into a regular pip-installable package:

pip install git+https://github.com/microsoft/satclip.git
from satclip.load import get_satclip            # pretrained location encoder
from satclip import SatCLIP, LocationEncoder     # or build models directly

Closes #8. Covers the core of #25; follow-ups from there (e.g. a src/ layout, PyPI release) can come as separate PRs.

Changes

  • pyproject.toml (setuptools, PEP 621): declares the runtime dependencies, Python ≥ 3.9, examples/dev extras, and ships configs/default.yaml as package data.
  • Package imports: intra-package imports are now relative (from .model import SatCLIP), and satclip/__init__.py exposes SatCLIP, LocationEncoder, SatCLIPLoss and get_satclip.
  • Training entry point: a satclip-train console script (equivalent to python -m satclip.main) uses the packaged default config, so it works from any directory: satclip-train --data.data_dir /path/to/s2-100k. The resolved config is saved to the run's log dir (satclip_logs/.../default-latest.yaml).
  • get_satclip: keeps only the hyperparameters SatCLIPLightningModule accepts. This lets it load both the released checkpoints and newly trained ones; before, the latter failed with KeyError/TypeError.
  • Tests + CI:
    • The tests cover import, model forward pass, and an end-to-end training → checkpoint → get_satclip round trip on synthetic S2 tiles.
    • The new GitHub Actions workflow builds the sdist and wheel, then tests the installed wheel from outside the source tree on Python 3.9 and 3.13.
  • Docs: the README has install and training instructions, and the notebooks install the package instead of cloning the repo and editing sys.path.

Testing

Each check below used a fresh venv and the built wheel (CPU torch), run from outside the repo:

Python 3.9 Python 3.12 Python 3.13
Resolved stack torch 2.8, torchgeo 0.5.2, lightning 2.6.0 torch 2.14, torchgeo 0.10.1, lightning 2.6.6 same as 3.12
pytest (5 tests, incl. e2e training) ✅ ✅ ✅
README examples (model forward, get_satclip with SatCLIP-ViT16-L40 from HF) ✅ ✅ ✅
load_lightweight with SatCLIP-ResNet18-L10 ✅ ✅ ✅
satclip-train --help ✅ ✅ ✅

Also checked:

  • pip install git+<repo>@packaging (build from source) followed by the README pretrained example.
  • An editable install plus python -m satclip.main --config custom.yaml on synthetic data.

Not tested: a full training run on the real S2-100K dataset, and GPU.

Thanks to Jean-Guillaume Durand (@jeangud) for the original PR and Monodrom (@ibaris) for the packaging work in #25.

🤖 Generated with Claude Code

Comment thread pyproject.toml
@@ -0,0 +1,39 @@
[build-system]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is the main change in this PR

@konstantinklemmer

Copy link
Copy Markdown
Collaborator

Thank you so much for this Jean-Guillaume Durand (@jeangud)! Just a quick note: I am taking some time off currently but will get to this as soon as I am back to working. Thanks for your contributions to this project.

Bring packaging branch up to date with latest main (README dataset/model
updates and notebook URL fixes).
Build on the packaging work with the follow-ups needed to make the package
usable end-to-end:

- docs/notebooks: migrate README quickstart and notebooks (A01/A02/B01/B02)
  off 'git clone + sys.path' to 'pip install' + 'from satclip... import ...';
  add an Installation section and run training via 'python -m satclip.main'
- pyproject: list direct deps explicitly (torch, numpy, sympy, timm, einops,
  matplotlib, torchvision, huggingface_hub), relax 'lightning' pin to >=2.2,<3,
  SPDX license, add 'examples'/'dev' extras and a 'satclip-train' entry point,
  ship configs/*.yaml as package data
- main.py: resolve the default config relative to the package and guard the
  CUDA device check so 'python -m satclip.main' works from any directory / on CPU
- __init__.py: expose a clean public API (SatCLIP, LocationEncoder,
  SatCLIPLoss, get_satclip) instead of wildcard re-exports
- add a CI workflow and import/forward-pass smoke tests
- ignore dist/ and .pytest_cache/

Verified: wheel builds, bundles configs/default.yaml, registers the
satclip-train entry point, and emits License-Expression: MIT.
@calebrob6

Copy link
Copy Markdown
Member

Konstantin Klemmer (@konstantinklemmer) what do you think about this? Still something you want?

@konstantinklemmer

Copy link
Copy Markdown
Collaborator

Yea I do; just haven't gotten around to it yet 🫠

…talled package

- Add tensorboard and jsonargparse[signatures] as dependencies; the default
  training config (TensorBoardLogger) and LightningCLI need them.
- Save the resolved training config into the run's log dir instead of next to
  the packaged default config (previously written into site-packages).
- Apply the A100 TF32 setting in cli_main so the satclip-train entry point
  gets it; never force TF32 off.
- get_satclip: keep only hyperparameters SatCLIPLightningModule accepts, so
  newly trained checkpoints load (legacy keys and Lightning's _instantiator).
- Add an end-to-end training + reload test on synthetic S2 tiles.
- CI: build sdist/wheel, test the installed wheel from outside the source
  tree on Python 3.9 and 3.13.
- README: document satclip-train; notebooks: drop redundant pip installs.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@konstantinklemmer

Copy link
Copy Markdown
Collaborator

Yo Caleb Robinson (@calebrob6), could you have a look? I also fixed:

  • training was missing the tensorboard dependency
  • the resolved config was written into site-packages
  • get_satclip couldn't load newly trained checkpoints
  • also added an end-to-end train-and-reload test, and CI now tests the built wheel on Python 3.9 and 3.13.
    If that looks OK to you, we can merge!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing requirements.txt file

3 participants