Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

interest in modernizing? #20

Open
jGaboardi opened this issue Mar 23, 2024 · 9 comments
Open

interest in modernizing? #20

jGaboardi opened this issue Mar 23, 2024 · 9 comments
Assignees
Labels

Comments

@jGaboardi
Copy link
Collaborator

@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
Copy link
Owner

I mean, yeh awesome. That would be amazing!

@jGaboardi
Copy link
Collaborator Author

jGaboardi commented May 25, 2024

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 – remove portion of setup.py to allow for install #23
  2. ✅ initialize a single py312_latest bare-bones environment + GHA for testing current package / code base - ensure passing – remove portion of setup.py to allow for install #23
    1. .github/workflows/testing.yml
    2. ci/py312_latest.yml - Ubuntu
  3. ✅ remove .travis.ymlremove old .travis.yml -- #20 #25
  4. ✅ consolidate old infrastructure into pyproject.tomlconsolidate infrastructure into pyproject.tml #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. add CODECOV_TOKEN secret #26
    2. update testing workflow file for CODECOV secret #27
    3. [INFRA] add a codecov.yml #28
  6. ✅ add ruff specifications to pyproject.toml for linting and formatting codebase – utilizing ruff for formatting & linting – #20 #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 – utilizing ruff for formatting & linting – #20 #31
  8. ✅ add a .pre-commit-configuration.yaml + turn on pre-commit for repo – add pre-commit-config.yaml - #20 #32
  9. ✅ review dependency min versions & fill out CI envs – Ubuntu / Windows/ macOS – fix #22 - update min req & boost CI suite #20 #33
    1. [COMPAT] fastpairs not compatible with scipy>=1.12 #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. start work on improving testing #50
    2. flesh out testing - #50 #61
  11. ✅ add a notebooks/ or examples/ directory with some demonstrations
    1. adding first notebook - basic usage #40
    2. notebooks: refactor basic - add n-dimensional #46
    3. ideas for more notebooks #45
    4. etc.
  12. 👀 add type hints – Add type hints - beef up docstrings - test examples, etc. #72
  13. 👀 revisit docstrings for numpydoc conformity – Add type hints - beef up docstrings - test examples, etc. #72
  14. ⚠️ ensure tests for all errors & warnings
    1. mixed dimensional errors
    2. min points warning (initialized via build()? #58)
    3. ...
    4. ...
  15. ⚠️ test against several distance scipy.spatial.distance functions
  16. ⚠️ test against custom distance function
  17. ➿ revisit README for updates, badges, etc. :
    1. update README by section #35
    2. add matplotlib to dependencies #36
    3. first update to README - formatting + Overview, Installation, Testing #37
    4. adding first notebook - basic usage #40
    5. boil down basic_usage.ipynb further - hard code points #41
    6. advanced_usage demo w/ high-dimensional points #44
    7. review wording, etc. in README
  18. ⚠️ (potential) see about docs/ + a GHPages site built from a GHA
  19. ⚠️ (potential) see about release GHA based on pushed v* tags (automated publishing on PyPI)
  20. ⚠️ Following release on PyPI, initialize release on conda-forge as a feedstock
    1. add PyPI & conda-forge badge (once available) #43
    2. start with an alpha, etc release sand iterate until all actions, etc are in order
  21. ⚠️ 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
Copy link
Owner

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
Copy link
Owner

Totally separate note: Is PySAL using this library?

@jGaboardi
Copy link
Collaborator Author

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.

@jGaboardi jGaboardi self-assigned this May 27, 2024
jGaboardi added a commit that referenced this issue May 28, 2024
* kicking off #20

* re-add version temporarily
jGaboardi added a commit that referenced this issue Jun 5, 2024
* initial ruff formating

* lint fastpair

* Update fastpair/test/test_fastpair.py

Co-authored-by: Carson Farmer <carson.farmer@gmail.com>

---------

Co-authored-by: Carson Farmer <carson.farmer@gmail.com>
jGaboardi added a commit to jGaboardi/fastpair that referenced this issue Jun 8, 2024
jGaboardi added a commit that referenced this issue Jun 10, 2024
* fix #22 - update min req & boost CI suite #20

* fix indent in testing.yml

* Update ci/py311-latest.yaml
@gegen07
Copy link

gegen07 commented Jun 11, 2024

@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
Copy link
Collaborator Author

@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
Copy link
Collaborator Author

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
Copy link

gegen07 commented Jun 13, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants