Skip to content

Modern Python Package Refactor (pyproject.toml, src layout, versioning) #25

Closed
Closed
Modern Python Package Refactor (pyproject.toml, src layout, versioning)#25

Description

@ibaris
Monodrom (ibaris)
opened on Mar 31, 2025

Hi,

I've been actively using Satclip and recently refactored it into a modern Python package here: Haight3/satclip. I wanted to ask whether you'd be interested in adopting this structure upstream or as an optional branch.

Here’s what was changed:

Key Modernization Features:

  • PEP 621-compliant pyproject.toml (removes the need for setup.py/setup.cfg)

  • src/ layout to improve import hygiene and avoid namespace clashes

  • Explicit versioning support baked into the build process

  • All imports cleaned and made absolute for clarity

  • Add Documentation to most of the functions and modules.

Benefits:

  • Easier integration with tools like pipx, build, tox, pytest, and ruff

  • Simplifies distribution via PyPI or internal registries

  • Much easier for downstream users to consume and extend

  • Plays well with IDEs and type checkers (VSCode, PyCharm, mypy)

  • Helps modularize and test parts like location_encoder, wrap, etc.

Happy to open a PR or break this into smaller parts if you think it’s of interest!

Activity

  1. elijahrockers commented on Apr 7, 2025

    @elijahrockers
    Eli (elijahrockers)
    on Apr 7, 2025

    This is related to #8 and #19 I believe... also hoping to see something like this implemented. I've tried to checkout a pre-trained model as per instructions in the README, on a linux based conda environment, but can't seem to get it working. It's not clear (to me, at least) what the requirements are. Konstantin Klemmer (@konstantinklemmer)

  2. ibaris commented on Apr 8, 2025

    @ibaris
    Monodrom (ibaris)
    on Apr 8, 2025
    Author

    This is related to #8 and #19 I believe... also hoping to see something like this implemented. I've tried to checkout a pre-trained model as per instructions in the README, on a linux based conda environment, but can't seem to get it working. It's not clear (to me, at least) what the requirements are. @konstantinklemmer

    You are totally right! I did not update the README properly and I have to put some work in the environment. I will do that this week and make some test to verify its functionality.

  3. Monodrom (ibaris)
    added a commit that references this issue on Apr 10, 2025

    feat(README): Update README.

    fff096a

  4. ibaris commented on Apr 10, 2025

    @ibaris
    Monodrom (ibaris)
    on Apr 10, 2025
    Author

    The issues have now been addressed here:

    Note: I haven’t been able to test the full training pipeline yet and would appreciate help verifying that everything still works smoothly on your end.

    The training can be started with the new entry point

    satclip --help

    Looking forward to your feedback!

  5. konstantinklemmer commented on Apr 11, 2025

    @konstantinklemmer
    Konstantin Klemmer (konstantinklemmer)
    on Apr 11, 2025
    Collaborator

    Hi Eli (@elijahrockers) and Monodrom (@ibaris) and sorry for the delay. As I unfortunately do not have the time to do a proper refactor / packaging myself, I would definitely be interested in potentially adopting your packaged version Monodrom (@ibaris) in here. Would you be willing to set up a PR and get that process started?

    Eli (@elijahrockers) could you let me know what exactly you struggle with in getting the pretrained models to work?

  6. calebrob6 commented on Apr 11, 2025

    @calebrob6
    Caleb Robinson (calebrob6)
    on Apr 11, 2025
    Member

    I think this is mostly handled here https://github.com/microsoft/satclip/pull/19/files

  7. konstantinklemmer commented on Apr 11, 2025

    @konstantinklemmer
    Konstantin Klemmer (konstantinklemmer)
    on Apr 11, 2025
    Collaborator

    Hmm maybe I should just merge #19 and Monodrom (@ibaris) can add additional changes in a separate PR? There's some extra things in https://github.com/Haight3/satclip as far as I can tell from a quick glance.

  8. Monodrom (ibaris)
    added a commit that references this issue on Apr 11, 2025

    fix(satclip.yml): Fix pytorch cuda issue

    97b422f

  9. ibaris commented on Apr 21, 2025

    @ibaris
    Monodrom (ibaris)
    on Apr 21, 2025
    Author

    Sorry for the delayed reply, and thank you so much for getting back to me!

    I really appreciate the openness. Instead of merging everything at once with a different PR, it would probably make more sense to contribute incrementally. I’d be happy to start by contributing to PR #19 and build from there!

  10. konstantinklemmer commented on Apr 30, 2025

    @konstantinklemmer
    Konstantin Klemmer (konstantinklemmer)
    on Apr 30, 2025
    Collaborator

    That would be fantastic Monodrom (@ibaris)! Please ping me once you have had a look at #19 and have integrated any potential adaptations you'd propose.

  11. elijahrockers commented on May 15, 2025

    @elijahrockers
    Eli (elijahrockers)
    on May 15, 2025

    Hi Konstantin Klemmer (@konstantinklemmer)

    @elijahrockers could you let me know what exactly you struggle with in getting the pretrained models to work?

    In the most recent attempt, I created a new conda environment, installed python=3.12, pytorch, and huggingface_hub. When I try to import get_satclip, it complains that I don't have the "lightning" module. So I conda installed the lightning module. Unfortunately, the installation of the "lightning" module breaks my pytorch installation with the following error message when I try to import it:

    Python 3.12.9 | packaged by Anaconda, Inc. | (main, Feb  6 2025, 18:56:27) [GCC 11.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import torch
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/toolbox/.conda/envs/satclip/lib/python3.12/site-packages/torch/__init__.py", line 367, in <module>
    from torch._C import *  # noqa: F403
    ^^^^^^^^^^^^^^^^^^^^^^
ImportError: /home/toolbox/.conda/envs/satclip/lib/python3.12/site-packages/torch/../../../libtorch_python.so: undefined symbol: _ZN5torch3jit5fuser6onednn9fuseGraphERSt10shared_ptrINS0_5GraphEE

    To fix it, I remove the entire environment, and reinstall pytorch.

  12. jeangud commented on Jun 29, 2025

    @jeangud
    Jean-Guillaume Durand (jeangud)
    on Jun 29, 2025
    Contributor

    I just noticed this thread. I agree with the suggestions Eli (@elijahrockers) and Monodrom (@ibaris)! Definitely feel free to take over #19 and add some commits there directly. Let me know if you need any support or help in this.

  13. Monodrom (ibaris)
    added a commit that references this issue on Mar 10, 2026

    Setup and Forward (#3)

    499ed84

  14. konstantinklemmer commented on Oct 8, 2026

    @konstantinklemmer
    Konstantin Klemmer (konstantinklemmer)
    on Oct 8, 2026
    Collaborator

    Addressed in #19.

  15. Konstantin Klemmer (konstantinklemmer)
    closed this as completedon Oct 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions