libpysal icon indicating copy to clipboard operation
libpysal copied to clipboard

libpysal.graph roadmap to release

Open martinfleis opened this issue 2 years ago • 33 comments

I tried to outline what is missing before we could cut a release with the new graph stuff. Open to a discussion.

testing

We have a decent coverage of base but especially constructors are not always tested for correctness. So we test the API but not that the adjacency captures what it should. So missing tests:

  • [x] test correctness of contiguity builders #566
  • [x] test correctness of triangulation builders
  • [x] test correctness of kernel builders
  • [x] test all kernel parameters
  • [x] test all query options (both sklearn and scipy)
  • [x] test precomputed distance matrix support (indirectly via triangulation and distance band already covered)

implementation/checks

Some bits are still waiting for implementation (I have likely missed stuff here) or a double check that the current implementation works as intended.

  • [x] isolates in kernel and knn
  • [x] co-located in knn
  • [x] knn haversine checks
  • [x] fix cliques
  • [x] set operations
  • [ ] draft API and raise NotImplementedError?

discussion

A few things I'd like to discuss (during the dev call in 3 minutes?)

  • [x] default kernel - W has triangular, we now have gaussian following scipy model - do we want to warn? how?
  • [x] ~expose kernels in transform? kernels are essentially transformations. Shall we allow to use them post-creation?~
  • [x] how should set operations work? See #561

follow-up

Stuff that is not implemented or started but that can likely wait for subsequent release, not the initial one. Below is the list of stuff I'd like to see soon but there's more to be done.

  • [x] lag_categorical (inside lag()?)
  • [x] classic I/O (we have custom Parquet now)
  • [x] plotting #593
  • [x] fuzzy contiguity #564
  • [ ] other utils - any priorities?
  • [x] optimal sparse formats (coo may be slow) #697
  • [x] IDs handling cleanup #626
  • [ ] symmetrize (see #664)
  • [x] fill diagonal #668
  • [x] higher-order neighbors
  • [x] trace

documentation

API reference should be in the docs for the first release. user guide can follow later.

  • [x] API reference
  • [x] user guide
  • [x] migration guide

Some of the follow-up could be piped via W for the time being if we want them available.

martinfleis avatar Sep 07 '23 15:09 martinfleis

Can you drop a note if you are planning to work on any of the topics mentioned above so we don't overlap?

martinfleis avatar Sep 13 '23 14:09 martinfleis

I am unlikely to have time until sept 27

ljwolf avatar Sep 13 '23 14:09 ljwolf

I think I’ll go back to ensuring the constructors can consume sparse next

knaaptime avatar Sep 13 '23 15:09 knaaptime

With #577, we're ready for the initial experimental release of Graph.

  • cliques are not fixed, but they raise NotImplementedError at the moment.
  • precomputed matrix is not tested explicitly but a few of our internal functions use it, so it is tested via those.
  • API is not yet drafted, but that does not need to be for a release
  • The default kernel is still Gaussian
  • set operations are implemented with some restrictions ensuring the validity of the Graph and its alignment with original data. See #575 for details.

Anything else you'd like to get in?

I would target the end of the week at the latest to cut 4.8.0 (need it alive on conda on Tuesday)

martinfleis avatar Sep 26 '23 20:09 martinfleis

Anything blocking the release now that #577 is merged?

@jGaboardi any idea if the current infrastructure works as intended? It has not been updated here.

martinfleis avatar Sep 29 '23 12:09 martinfleis

I am not sure. Let's give it try; perhaps as a 4.7.0.post1 release?

jGaboardi avatar Sep 29 '23 12:09 jGaboardi

More like 4.8.0rc1 given the status of current main

martinfleis avatar Sep 29 '23 13:09 martinfleis

@jGaboardi Action looking good - https://github.com/pysal/libpysal/actions/runs/6353690175/job/17258825775

PyPI is up - https://pypi.org/project/libpysal/4.8.0rc1/

Changelog is completely broken - https://github.com/pysal/libpysal/releases/tag/v4.8.0rc1

martinfleis avatar Sep 29 '23 15:09 martinfleis

Documentation is okay - https://pysal.org/libpysal/

martinfleis avatar Sep 29 '23 15:09 martinfleis

Changelog is completely broken

Yeah, looks like the action still uses tools/gitcount.ipynb. This will need to be looked it if we still want to use it.

spaghetti has been using actions/github-script@v6 with success, if we want to try here.

jGaboardi avatar Sep 29 '23 15:09 jGaboardi

lets update the root and get rid of versioneer etc before doing a real release

knaaptime avatar Sep 29 '23 18:09 knaaptime

@knaaptime Are you on it or shall I?

martinfleis avatar Sep 29 '23 18:09 martinfleis

i'll take the first pass

knaaptime avatar Sep 29 '23 18:09 knaaptime

Unless anyone does that first, I'm happy to cut 4.8.0 later tonight (Prague time) or tomorrow morning.

martinfleis avatar Oct 01 '23 15:10 martinfleis

Unless anyone does that first, I'm happy to cut 4.8.0 later tonight (Prague time) or tomorrow morning.

Seem to have a strange merge problem with upstream/main vs. my main. Both docs build and publish failed. Going to delete the v4.8.0 tag and try to figure it out.

jGaboardi avatar Oct 01 '23 18:10 jGaboardi

Seems like two key pieces slipped through the cracks:

  1. Updated method for install deps for creating release.
  2. This line seems to be missing in libpysal/docs/conf.py

I'll put in the necessary PR shortly.

jGaboardi avatar Oct 01 '23 18:10 jGaboardi

we dont need the sys path hack

knaaptime avatar Oct 01 '23 18:10 knaaptime

i think we're using an old recipe for the docs. It should have an install line before making the docs (so you get the real version without doing the sys/path hack). I think there's a current version in tobler

knaaptime avatar Oct 01 '23 18:10 knaaptime

we dont need the sys path hack

Seems to fail without it. ~~Is there something I am missing?~~ Yes, there was. LOL

jGaboardi avatar Oct 01 '23 18:10 jGaboardi

@knaaptime @martinfleis

Can confirm docs build locally, but with the following warnings:

libpysal/libpysal/cg/shapes.py:docstring of libpysal.cg.LineSegment.bounding_box:1:<autosummary>:1: WARNING: Inline strong start-string without end-string.
libpysal/libpysal/cg/shapes.py:docstring of libpysal.cg.LineSegment.bounding_box:1:<autosummary>:1: WARNING: Inline strong start-string without end-string.
libpysal/docs/notebooks/fetch.ipynb:4: WARNING: Each notebook should have at least one section title
libpysal/docs/notebooks/io.ipynb:4: WARNING: Each notebook should have at least one section title
libpysal/docs/notebooks/Raster_awareness_API.ipynb: WARNING: document isn't included in any toctree
libpysal/docs/notebooks/fetch.ipynb: WARNING: document isn't included in any toctree
libpysal/docs/notebooks/io.ipynb: WARNING: document isn't included in any toctree
  • The first two, I can't find anything to actually fix after finding some possible solution.
  • The second two are "whatevers".
  • For the final three, are we not including those in tutorial.rst on purpose? If so, then nothing to fix.

jGaboardi avatar Oct 01 '23 19:10 jGaboardi

I we are OK with that, I will push up the fixes immediately.

jGaboardi avatar Oct 01 '23 19:10 jGaboardi

Release failed, I think because there is no README.md in libpysal... it's README.rst.

jGaboardi avatar Oct 01 '23 19:10 jGaboardi

@knaaptime @martinfleis

v4.8.0rc2 is tagged and up on PyPI with docs built successfully. Please give a once over. If everything looks kosher, I will cut v4.8.0.

jGaboardi avatar Oct 01 '23 20:10 jGaboardi

All looking good to me!

martinfleis avatar Oct 01 '23 20:10 martinfleis

Do you or Eli want to do the honors since yall put in that hard work? Or shall I go ahead and cut it?

jGaboardi avatar Oct 01 '23 20:10 jGaboardi

Go ahead 😉

martinfleis avatar Oct 01 '23 20:10 martinfleis

The decision on ID API has been to allow passing an array of IDs when the input is a sparse or dense precomputed array and raise a ValueError when it is passed alongside a data frame.

martinfleis avatar Nov 02 '23 16:11 martinfleis

xref #712

jGaboardi avatar Jun 10 '24 19:06 jGaboardi