holoviz / datashader

Quickly and accurately render even the largest data.
http://datashader.org
BSD 3-Clause "New" or "Revised" License
3.32k stars 365 forks source link

Curate dependencies #1116

Closed ianthomas23 closed 2 years ago

ianthomas23 commented 2 years ago

Closes #1112.

Significant changes here are:

  1. spatialpandas is not a compulsory dependency (it hasn't been for a while) so I have modified the import spatialpandas code to gracefully handle the situation of it not being available.
  2. I've removed colorcet as a compulsory dependency as it is not used anywhere in the code other than the tests and examples. I think this is the correct approach, but it may impact downstream projects which depend on it being included with datashader.
  3. I've added a github action to test using pip (via the pyctdev doit ecosystem=pip approach) so that we are automatically testing both pip and conda installations.

I also removed bcolz as it is no longer maintained (https://github.com/Blosc/bcolz) but it is only used in filetimes.py. There are other possible removals here but I intend to replace the benchmarking code with a full asv benchmarking suite so I will address the other filetimes.py changes then.

codecov[bot] commented 2 years ago

Codecov Report

Merging #1116 (92126a9) into master (99d5476) will decrease coverage by 0.10%. The diff coverage is 54.83%.

@@            Coverage Diff             @@
##           master    #1116      +/-   ##
==========================================
- Coverage   85.07%   84.96%   -0.11%     
==========================================
  Files          34       34              
  Lines        7516     7531      +15     
==========================================
+ Hits         6394     6399       +5     
- Misses       1122     1132      +10     
Impacted Files Coverage Δ
datashader/tiles.py 69.50% <0.00%> (ø)
datashader/glyphs/points.py 88.29% <25.00%> (-1.82%) :arrow_down:
datashader/glyphs/line.py 93.45% <50.00%> (-0.19%) :arrow_down:
datashader/glyphs/polygon.py 94.80% <50.00%> (-1.20%) :arrow_down:
datashader/core.py 87.71% <84.61%> (-0.40%) :arrow_down:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

jbednar commented 2 years ago

I've been trying to decide how I feel about removing colorcet as a dependency. Yes, obviously, the Datashader code does not depend on colorcet, so it should be possible to install without colorcet when that's desired. On the other hand, colorcet was created very specifically to address a problem faced by Datashader users, namely that typical colormaps undo all the principled, accurate data rendering that Datashader does. I want to be able to freely use colorcet throughout Datashader examples, without having to explain to users that they need to install it. I also want to specifically encourage Datashader users to use colorcet freely themselves, without requiring some explicit action on their part to make it available.

With that in mind, we could make colorcet an optional dependency instead, available by default but avoidable. On conda that currently requires splitting datashader into two packages, datashader-core and datashader. But unless we have other reasons to make such a split, I don't see a compelling reason to incur the maintenance cost of having the two separate packages, given how small and simple colorcet is. So on balance, unless we have other convincing reasons to maintain two packages, I think we should restore colorcet as a dependency.

ianthomas23 commented 2 years ago

Yes, your argument makes a lot of sense. It is not as if colorcet has a complicated set of dependencies anyway. I'll put it back in as a compulsory dependency.