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

pass kwargs to explore #627

Merged
merged 1 commit into from
Nov 3, 2023
Merged

Conversation

martinfleis
Copy link
Member

Follow-up on #617

We were not able to change anything about the map as we forgot to pass kwargs though like we did in esda. But stuff like tiles or prefer_canvas shall work out of the box.

@knaaptime
Copy link
Member

cool. The way i'd handled that before was to just cheat and inculude extra args like tiles in the kwargs for one of the earlier maps (like stick them in the edge_kws). This is definitely preferable

@martinfleis
Copy link
Member Author

A side note - I also realised that we can directly plot weight. Neat.
Screenshot 2023-11-03 at 23 08 10

@knaaptime
Copy link
Member

oh dope!

@martinfleis
Copy link
Member Author

The way i'd handled that before was to just cheat and inculude extra args like tiles in the kwargs for one of the earlier maps

Yeah but that is very unintuitive and requires you to know the implementation.

@martinfleis martinfleis added enhancement rough edge Something that's not a bug, but that gets in the way of "it just works." and removed enhancement labels Nov 3, 2023
@knaaptime
Copy link
Member

knaaptime commented Nov 3, 2023

Yeah but that is very unintuitive and requires you to know the implementation.

💯

A side note - I also realised that we can directly plot weight. Neat.

does that mean that works for the static plots too?

@martinfleis
Copy link
Member Author

martinfleis commented Nov 3, 2023

does that mean that works for the static plots too?

No, because we don't reuse GeoDataFrame.plot there but build collections directly. Going via GeoPandas would cause significant performance hit. We could potentially map colours ourselves though...

Copy link

codecov bot commented Nov 3, 2023

Codecov Report

Merging #627 (9c17c03) into main (4290c11) will increase coverage by 0.0%.
The diff coverage is 100.0%.

Impacted file tree graph

@@          Coverage Diff          @@
##            main    #627   +/-   ##
=====================================
  Coverage   84.5%   84.5%           
=====================================
  Files        139     139           
  Lines      14963   14967    +4     
=====================================
+ Hits       12638   12642    +4     
  Misses      2325    2325           
Files Coverage Δ
libpysal/graph/_plotting.py 92.9% <100.0%> (ø)
libpysal/graph/base.py 97.6% <ø> (ø)
libpysal/graph/tests/test_plotting.py 100.0% <100.0%> (ø)

@martinfleis martinfleis merged commit 6072541 into pysal:main Nov 3, 2023
10 checks passed
@martinfleis martinfleis deleted the explore_kwags branch November 3, 2023 22:36
@martinfleis
Copy link
Member Author

@knaaptime would you like to cut 4.9.1? :)

@knaaptime
Copy link
Member

knaaptime commented Nov 3, 2023 via email

@martinfleis
Copy link
Member Author

@knaaptime I suggest we first figure out what caused this conda-forge error https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=817319&view=logs&j=656edd35-690f-5c53-9ba3-09c10d0bea97&t=986b1512-c876-5f92-0d81-ba851554a0a3

cc @jGaboardi

It is midnight here so that is not a task for me today :)

@knaaptime
Copy link
Member

so, i dunno why thats failing. The error seems to come from xarray not being available when weights is imported, but afaict xarray is never imported in the weights module except inside a function-level 'try' block.

...that said, it looks like we do define xarray as a hard dependency in pyproject.toml, but not in the conda forge recipe. A quick fix would be to include xarray in conda, but do we need it? Also not clear why the error is actually materializing here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
graph rough edge Something that's not a bug, but that gets in the way of "it just works."
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants