carsonfarmer / fastpair

FastPair: Data-structure for the dynamic closest-pair problem.
MIT License
12 stars 4 forks source link

interest in modernizing? #20

Open jGaboardi opened 6 months ago

jGaboardi commented 6 months ago

@carsonfarmer Do you have any interest in seeing fastpairs modernized (e.g., Python 312 compatibility, pyproject.toml, GHA for testing, structured formatting & linting, etc.)? If so (and as time permits), I can start making specific tickets and chipping away at it.

xref: #12

carsonfarmer commented 4 months ago

I mean, yeh awesome. That would be amazing!

jGaboardi commented 4 months ago

Plan of attack

  1. having trouble with local install due non-conforming versioning (see here). seems to need some minor doctoring and potential implementation of (4.v) before continuing. will investigate – #23
  2. ✅ initialize a single py312_latest bare-bones environment + GHA for testing current package / code base - ensure passing – #23
    1. .github/workflows/testing.yml
    2. ci/py312_latest.yml - Ubuntu
  3. ✅ remove .travis.yml – #25
  4. ✅ consolidate old infrastructure into pyproject.toml – #30
    1. setup.py
    2. requirements.txt
    3. recommended.txt
    4. .coveragerc
    5. use setuptools-scm for versioning
  5. ✅ add codecov.yml and turn on for the repo
    1. 26

    2. 27

    3. 28

  6. ✅ add ruff specifications to pyproject.toml for linting and formatting codebase – #31
    1. One PR but in the following order:
      1. format with ruff specifications
      2. push for prelim review
      3. lint with ruff specifications
      4. push for final review and iterate if needed
  7. ✅ remove Python 2 adaptations in the code base – #31
  8. ✅ add a .pre-commit-configuration.yaml + turn on pre-commit for repo – #32
  9. ✅ review dependency min versions & fill out CI envs – Ubuntu / Windows/ macOS – #33
    1. 22

    2. update pyproject.toml
    3. py310_oldest (based on SPEC000)
    4. py311_latest
    5. py312_dev
  10. ✅ fill out testing with any edge cases that crop up
    1. 50

    2. 61

  11. ✅ add a notebooks/ or examples/ directory with some demonstrations
    1. 40

    2. 46

    3. 45

    4. etc.
  12. 👀 add type hints – #72
  13. 👀 revisit docstrings for numpydoc conformity – #72
  14. :warning: ensure tests for all errors & warnings
    1. mixed dimensional errors
    2. min points warning (#58)
    3. ...
    4. ...
  15. :warning: test against several distance scipy.spatial.distance functions
  16. :warning: test against custom distance function
  17. :loop: revisit README for updates, badges, etc. :
    1. 35

    2. 36

    3. 37

    4. 40

    5. 41

    6. 44

    7. review wording, etc. in README
  18. :warning: (potential) see about docs/ + a GHPages site built from a GHA
  19. :warning: (potential) see about release GHA based on pushed v* tags (automated publishing on PyPI)
  20. :warning: Following release on PyPI, initialize release on conda-forge as a feedstock
    1. 43

    2. start with an alpha, etc release sand iterate until all actions, etc are in order
  21. :warning: Add add Release, PyPI, and conda-forge badges to README
  22. ...
  23. triage
  24. ...
  25. Proceed with first stable release of fastpair 🥳 🎉

@carsonfarmer This is a loosely sequential checklist of proposed activities. Some make sense to do in individual PRs and some may be grouped. Let me know if you have any questions or concerns with anything here, otherwise I'll see about getting started with (1.)

carsonfarmer commented 4 months ago

TL;DR: This all looks good to me!

I'm pretty unfamiliar with modern Python development flows at this stage @jGaboardi, so I would defer to your judgement here. Since I have, in fact, met you in person, I have done ahead and invited you to be a collaborator on this repo. You should have direct access to GitHub Actions etc as needed with that invite?

carsonfarmer commented 4 months ago

Totally separate note: Is PySAL using this library?

jGaboardi commented 4 months ago

Totally separate note: Is PySAL using this library?

Not currently, but I became aware of fastpair last summer when @ljwolf, @gegen07, and I were working with a student in a PySAL GSoC project for spopt. So there's potential for inclusion later down the line in our new KNearestPMedian model, but we'll need to do performance testing, benchmarking, etc. before that happens. So, my contributions here are for 3 reasons (in order of general to specific):

  1. I love the packaging aspect of the Scientific Python ecosystem.
  2. I believe your work on this has a lot value and making it available to a broader audience will hopefully make others' lives easier.
  3. The specific pysal/spopt reason mentioned above.
gegen07 commented 3 months ago

@jGaboardi I saw a lot of work done by you! Awesome! Any help wanted? The spopt will be helped with this brilliant implementation, thanks @carsonfarmer!

jGaboardi commented 3 months ago

@carsonfarmer For context here, @gegen07 was an outstanding PySAL GSoC 2021 student several years back that brought the locate module into existence, and he has since joined us on the core team.

jGaboardi commented 3 months ago

Any help wanted?

Of course! Your insight is always valuable, specifically due to your CS background. Anything specific that has caught your eye in the issues?

I can't speak for @carsonfarmer but IMHO we should get fastpair modernized with an actual release before addressing enhancements like:

gegen07 commented 3 months ago

No problem with current issues. All good! I'll take a look at the issues for in-depth enhancements. Agree with you, before enhancing it, let's get it over the release cycle first.