VowpalWabbit / coba

Contextual bandit benchmarking
https://coba-docs.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
48 stars 19 forks source link

[ci] run examples to avoid stale content #13

Closed lalo closed 2 years ago

codecov[bot] commented 2 years ago

Codecov Report

Merging #13 (81765d1) into master (26df519) will increase coverage by 0.00%. The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #13   +/-   ##
=======================================
  Coverage   99.80%   99.80%           
=======================================
  Files          47       47           
  Lines        3599     3601    +2     
=======================================
+ Hits         3592     3594    +2     
  Misses          7        7           
Flag Coverage Δ
unittest 99.80% <100.00%> (+<0.01%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
coba/experiments/results.py 100.00% <100.00%> (ø)
coba/contexts/cachers.py 100.00% <0.00%> (ø)
coba/environments/simulated/readers.py 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 26df519...81765d1. Read the comment docs.

lalo commented 2 years ago

@mrucker any idea on why this is failing? That import seems to be too deep in, was it recently moved?

mrucker commented 2 years ago

@lalo Hey man! Welcome back from the holidays. There's been a fair number of changes around here :). Hopefully nothing too bad. I'm not sure what you are referring to? Oh I see now.. Yeah, so I've been working on creating the first really good online documentation for the project (https://coba-docs.readthedocs.io/en/latest/). Sphinx and autodocs can be a little temperamental when creating automatic summaries so I changed some of the module structures to get it to play nicer (one of those changes was creating deeper modules in the environments module). I've learned a lot more now about Sphinx, and the deeper modules could probably be made flat again, but I just kind of left them in their new home for now. To be honest, I kind of don't like it but...

lalo commented 2 years ago

Hi @mrucker hope you had a good holiday break. I meant deep in the sense that it doesn't look like an install related failure but some hanging reference to something that got moved, I was surprise to see it fail since the test coverage is so high already. I don't have any strong opinion on how nested things are, as long as they make sense.

mrucker commented 2 years ago

@lalo Oh yeah, I see what you are saying... Yes, it is strange. If I'm looking at this correctly this is failing when First Script.py is run right and not on any specific unit test? And I did have a good break. I've spent most of my time trying to knock off some to-do items on my list with Coba before classes and research starts up again.

mrucker commented 2 years ago

@lalo Eventually one of these orders has to work, right? :D

lalo commented 2 years ago

@mrucker nice, glad to hear you are doing good. Yes, it is being triggered by the First Script under examples, and not the unit tests. I'm not sure if the order is the problem, or what is going on :\

mrucker commented 2 years ago

@lalo I've been able to duplicate this error locally. Have you?

mrucker commented 2 years ago

@lalo I figured it out! It's a bug in setup.py. I didn't add the new modules to the packages. Maybe the project is getting complex enough now we should graduate to a package finder instead of manually writing them all. When doing a development install it doesn't pick it up since all the source code is there. It only shows up when doing a release install.

lalo commented 2 years ago

I just did, I was on old coba commit.

mrucker commented 2 years ago

@lalo Yo, I went through and fixed all the broken Jupyter notebooks. The CI on the notebooks is amazing. This does cause/bring-up a small problem/question. You can load up these notebooks in Binder and Binder will download coba from pypi which will be behind the repo.... Do you think we should have Binder install coba from the repo? Or something else?

lalo commented 2 years ago

That could be a good idea, I think we might have binder or something similar for the VW repo already but not sure. Thanks for updating the notebooks.

mrucker commented 2 years ago

@lalo Oh, we actually already have bindr set up for Coba. It's been set up for a long time now (maybe 9 months?). I just recently added a link for it at the top of the readme. Setting up Binder was actually super easy if you ever want to do it for your own projects.