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

pytest on gh actions #66

Closed
wants to merge 1 commit into from
Closed

pytest on gh actions #66

wants to merge 1 commit into from

Conversation

kcpevey
Copy link
Collaborator

@kcpevey kcpevey commented Mar 8, 2021

@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). 

* comment out travis for testing

* add pytest-mpl to ci

* add pytest-mpl to test_unit_extras
@kcpevey
Copy link
Collaborator Author

kcpevey commented Mar 19, 2021

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
Copy link
Member

maximlt commented Sep 15, 2021

@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:

  • I had to pin jupyter_client to <7 due to this error TypeError: 'coroutine' object is not subscriptable. We're not the only one having this issue (Release 6.1.13 nbconvert failing with TypeError: 'coroutine' object is not subscriptable jupyter/jupyter_client#637). Note that in my dev env I have got the 5.5.0 version of nbconvert.
  • There was an error while running the user_guide/index.ipynb notebook: IndexError: string index out of range. It's because name='' leads to hv.Image(..., group=``) which raises the error in holoviews. It can simply be reproduced with hv.Image(np.ones((2, 2)), group='').

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

@maximlt
Copy link
Member

maximlt commented Sep 15, 2021

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
Copy link
Member

jbednar commented Sep 15, 2021

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
Copy link
Member

maximlt commented Oct 26, 2021

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

@maximlt maximlt closed this Oct 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants