Skip to content

Conversation

@weikang9009
Copy link
Member

@weikang9009 weikang9009 requested a review from jGaboardi December 3, 2025 18:50
@codecov
Copy link

codecov bot commented Dec 3, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.2%. Comparing base (3fa6225) to head (2885d8f).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main    #246     +/-   ##
=======================================
- Coverage   88.2%   88.2%   -0.1%     
=======================================
  Files         10      10             
  Lines       1591    1584      -7     
=======================================
- Hits        1404    1397      -7     
  Misses       187     187             
Files with missing lines Coverage Δ
giddy/plotting.py 92.4% <100.0%> (-0.3%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jGaboardi
Copy link
Member

@weikang9009 awesome!

Should we also tie in these other splot removals in this PR?

  • environment.yml
  • pyproject.toml
  • and the ci/*.yml files

@weikang9009
Copy link
Member Author

Yes! I thought those were removed - sorry for misreading and closing that issue.

I've just pushed another commit to this PR to remove splot from those files.

@weikang9009
Copy link
Member Author

One of the CI testing failed because of https://github.com/pysal/giddy/blob/main/giddy/plotting.py#L591 I think esda has this plotting functionality? Since esda is a dependency, we can replace it with the esda function.

@jGaboardi
Copy link
Member

One of the CI testing failed because of https://github.com/pysal/giddy/blob/main/giddy/plotting.py#L591 I think esda has this plotting functionality? Since esda is a dependency, we can replace it with the esda function.

esda>=2.7 has the functionality, so if simply bump the minimum esda version to 2.7 we can clean up that logic block nicely.

Thoughts?

@weikang9009
Copy link
Member Author

weikang9009 commented Dec 3, 2025 via email

@jGaboardi
Copy link
Member

I'll see about making those changes in your branch here and push back up.

Copy link
Member

@jGaboardi jGaboardi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@weikang9009 – we needed to bump the minimum libpysal, as well -- but now we have fully green CI 🎉

I'm approving, but I'll wait for you to look over my changes and merge (since I can't tag you as reviewer on your own PR).

@weikang9009 weikang9009 merged commit dab90b3 into pysal:main Dec 4, 2025
11 checks passed
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