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

Add the k nearest p-median module and the tutorial example for capacitated p-median #397

Merged
merged 17 commits into from
Oct 3, 2023

Conversation

rongboxu
Copy link
Collaborator

Hi all! This pull request originates from the feature branch and includes all the commits from PR #387 . It can be identified as the outcome of the GSOC 2023 project.

@rongboxu
Copy link
Collaborator Author

Hi all! I found this PR can't pass the CI test for python 3.9, 3.10 and 3.11, while it can pass the test for python 3.8.
The error happens when running the build_best_tree from pointpats.

The error message is:
if metric in KDTree.valid_metrics: TypeError: argument of type 'builtin_function_or_method' is not iterable.
I found there is one similar issue with this function.

I suppose it may be the version problem of sklearn which the build_best_tree function doesn't correctly define. However, I may need more time to identify the reason.

@jGaboardi
Copy link
Member

I am not sure it is a scikit-learn compatibility issue, because current pointpats testing passes with scikit-learn==1.3.0. Did tests pass locally for you?

.ci/310.yaml Outdated Show resolved Hide resolved
.ci/311-DEV.yaml Outdated Show resolved Hide resolved
.ci/311.yaml Outdated Show resolved Hide resolved
.ci/39.yaml Outdated Show resolved Hide resolved
@rongboxu
Copy link
Collaborator Author

I am not sure it is a scikit-learn compatibility issue, because current pointpats testing passes with scikit-learn==1.3.0. Did tests pass locally for you?

My local python is 3.8 version, so the test can pass.

rongboxu and others added 4 commits August 22, 2023 12:02
Co-authored-by: James Gaboardi <[email protected]>
Co-authored-by: James Gaboardi <[email protected]>
Co-authored-by: James Gaboardi <[email protected]>
Co-authored-by: James Gaboardi <[email protected]>
@rongboxu
Copy link
Collaborator Author

I am not very sure, but maybe this can be the reason of the failing of build_best_tree, see this.
The relevant code in the Github is, if metric in kdtree_valid_metrics:, see here.
While, in the test detail, it is if metric in KDTree.valid_metrics:, see here.

@gegen07
Copy link
Member

gegen07 commented Aug 22, 2023

I think pointpats was not released after this bug fix by Martin, then it throws this error. I think we should release it first to merge this pull request.

@jGaboardi
Copy link
Member

I think pointpats was not released after this bug fix by Martin, then it throws this error. I think we should release it first to merge this pull request.

Correct, @ljwolf mentioned that pysal/pointpats#105 needs to be merged prior to a fresh release of pointpats.

(trigger a CI run)
@jGaboardi
Copy link
Member

@gegen07 @ljwolf , with the new pointpats release all tests are passing except the known AZP failure. I think we are good to merge this. Any dissenting opinions?

@gegen07
Copy link
Member

gegen07 commented Oct 3, 2023

I think it is good to go.

@jGaboardi jGaboardi merged commit 3f284d9 into pysal:main Oct 3, 2023
2 of 8 checks passed
@jGaboardi
Copy link
Member

Congrats, @rongboxu!

@rongboxu
Copy link
Collaborator Author

rongboxu commented Oct 3, 2023

Congrats, @rongboxu!

Thank you very much for merging it!

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

Successfully merging this pull request may close these issues.

3 participants