Update kernel regex - Githubissues

anaconda / nb_conda

Conda environment and package access extension from within Jupyter

BSD 3-Clause "New" or "Revised" License

140 stars 32 forks source link

Update kernel regex #28

Closed bollwyvl closed 8 years ago

bollwyvl commented 8 years ago

For #27.

This updates:

the notebook conda dialog to correctly identify the current environment, based on nb_conda_kernels 2.0.0
some minor visual fixes
some more basic tests
uses requirements.txt instead of environment.yml for tests, etc.

coveralls commented 8 years ago

Coverage decreased (-0.6%) to 49.355% when pulling 809a64b6d6cffddb86bf2f528fb94993e262c5d2 on update-kernel-regex into 2b520527831aae1022af7ae7b8bb8447913ee625 on master.

bollwyvl commented 8 years ago

Blocked by: https://github.com/conda-forge/tornado-feedstock/pull/7

coveralls commented 8 years ago

Coverage remained the same at 50.0% when pulling 7f6e956ed2bb9f1f1dc56107bc05e08b0a6cb2ab on update-kernel-regex into 2b520527831aae1022af7ae7b8bb8447913ee625 on master.

coveralls commented 8 years ago

Coverage increased (+25.6%) to 75.628% when pulling b6378153bc42a03e477e66ca35628dd81e7b38a2 on update-kernel-regex into 2b520527831aae1022af7ae7b8bb8447913ee625 on master.

coveralls commented 8 years ago

Coverage increased (+25.6%) to 75.628% when pulling b6378153bc42a03e477e66ca35628dd81e7b38a2 on update-kernel-regex into 2b520527831aae1022af7ae7b8bb8447913ee625 on master.

bollwyvl commented 8 years ago

Hot dang, all 💚 ! Actually went and checked the appveyor builds, as i didn't really believe it... whaddya know, lots of ".trash ain't empty" messages. Just like the bad old days. But everything worked. So I'll take it.

Review love appreciated, @damianavila @mmarchetti @malev or anybody else you guys can think of that have spent time on this on. ended up being rather large.

mmarchetti commented 8 years ago

I did a brief review... +1 on the addition of tests!

damianavila commented 8 years ago

Quick overview as well, but this look nice... I like some of the simplifications I saw and obviously the tests are a great addition. Wondering why you decide to use the APIHandler + Mixin instead of continuing with the EnvHandler. I like the approach you took but this is something derived from other work, or other extensions you wrote/saw or just you saw the opportunity to make this way and did it?

bollwyvl commented 8 years ago

Wondering why you decide to use the APIHandler + Mixin instead of continuing with the EnvHandler

Kind of a leftover... i was looking at whether there needed to be a separate handler for some of the end points, but it didn't. I'll simplify it back down.

coveralls commented 8 years ago

Coverage increased (+26.8%) to 76.808% when pulling 706889407578fa36ccfb70e664ebef4aece35cbf on update-kernel-regex into 2b520527831aae1022af7ae7b8bb8447913ee625 on master.

coveralls commented 8 years ago

Coverage increased (+26.8%) to 76.808% when pulling cd2d9dd535d3008ab8c8d73ed0a463777f57782d on update-kernel-regex into 2b520527831aae1022af7ae7b8bb8447913ee625 on master.

bollwyvl commented 8 years ago

Ok, back to the simpler inheritance pattern! __mro__-begone!

bollwyvl commented 8 years ago

Ok, liking this! I'm going to merge and tag!