cytoscape / ipycytoscape

A Cytoscape Jupyter widget
https://ipycytoscape.readthedocs.io/en/master/
BSD 3-Clause "New" or "Revised" License
264 stars 62 forks source link

Add pre-commit config + update eslint #300

Closed ianhi closed 2 years ago

ianhi commented 2 years ago

Should fix: https://github.com/cytoscape/ipycytoscape/issues/299

@marimeireles do you think the instructions on usage are through enough? I can add more - could also imagine adding instructions for what to do once the bot pushes to your PR.

I expect the checks to fail as this repo has some flake8 issues (which we could ignore by removing the flake8 check)

github-actions[bot] commented 2 years ago

Binder :point_left: Launch a binder notebook on branch ianhi/ipycytoscape/pre-commit

ianhi commented 2 years ago

oh boy: image

ianhi commented 2 years ago

The failing test is one that was actually never being run before and that I discovered wasn't being due to the flake8 checks. Confused by the python version dependency though

marimeireles commented 2 years ago

This is such a weird error 😅 Can't reproduce it locally What's up with these CIs

marimeireles commented 2 years ago

I reran the test and now another python is unhappy for another weird reason.

jenshnielsen commented 2 years ago

I did a bit of experimenting on https://github.com/jenshnielsen/ipycytoscape/pull/4 and it looks like the test failure can indeed be reproduced by only renameing the second test_add_edges to avoid collisions. It may be worthwile extracting that from this pr marking it as xfail and then merge the rest of this and fix it in a subsequent pr ?

jenshnielsen commented 2 years ago

308 should resolve the test issue while still running all the tests

ianhi commented 2 years ago

https://github.com/cytoscape/ipycytoscape/pull/308 should resolve the test issue while still running all the tests

Thanks @jenshnielsen ! just rebased