holoviz / colorcet

A set of useful perceptually uniform colormaps for plotting scientific data
http://colorcet.holoviz.org
Other
682 stars 52 forks source link

pytest on gh actions #66

Closed kcpevey closed 2 years ago

kcpevey commented 3 years ago

@jbednar @philippjfr I could use some feedback on this PR. I believe pytest was only run on Linux 3.7. I've opened it up to all platforms/versions so its possible that some of these are known issues that I should avoid.

Current status:
Python 3.6 - PASS
Python 3.7 - PASS

Python 2.7, Ubuntu
Some failures looking for DISPLAY env var: Invalid DISPLAY variable  https://github.com/kcpevey/colorcet/pull/1/checks?check_run_id=2042433951#step:9:489 Which I could set... but if you look at the code block where the failure is, its thinking that the ubuntu 2.7 build is X11 and its part of a qt5 backend. Which just seems wrong is several different ways. Should I just set the variable it wants or dig into why its getting there in the first place?

I see this failure on all 2.7 architectures https://github.com/kcpevey/colorcet/pull/1/checks?check_run_id=2042433951#step:9:502 Seems like the codebase is not actually supported on 2.7? It would be an easy fix to swap these around though. Should add that in as well?

For python 3.5 I'm seeing this:


  target environment location: C:\Miniconda3\envs\colorcet
  current conda version: 4.5.11
  minimum conda version: 4.9

I did some initial googling about this, but I haven't worked out a solution (or what the problem is, actually). 

kcpevey commented 3 years ago

After discussion with the holoviz dev team, it was decided that I will fix the py 2.7 issues (which are only related to the handy visualization tools, not using the library in general). I will also drop py 3.5 testing.

maximlt commented 3 years ago

@jbednar happy to see that there's work already done to migrate to GHA in this PR :)

I ran the test suite locally on Python 3.7 and it failed for two reasons:

Is there any update on py 2.7? Still supported and the bug reported in the original post still has to be fixed?

maximlt commented 3 years ago

Note that instead of pinning jupyter_client<2, pinning nbconvert>5 works too. I still have to figure out why I got a version 5 of nbconvert in the first place, the current version being 6.1.0.

jbednar commented 3 years ago

I don't expect to remove python2 support from colorcet itself unless there is a really compelling reason. It has so little code that supporting both platforms seems like very little cost, and there's no reason that I know of that people shouldn't be able to use it from python2 indefinitely.

That said, I think our python2 tests should be minimal, testing the colormaps themselves and not any other auxiliary code, docs, etc. That way python2 users won't see their code break, but we don't promise anything else.

I'm not able to follow Kim's links above to what the specific Python2 issues are, but the DISPLAY one seems similar to earlier issues with matplotlib defaulting to wxAgg or some other display backend that requires a live DISPLAY; it should always be set to Agg for the test suite. That might require an environment variable or other global setting.

maximlt commented 2 years ago

Closing since the CI now runs on GHA. Thanks a lot @kcpevey for starting this, it helped me a lot!