earthlab / matplotcheck

A python package for checking and testing matplotlib plots. We use this for autograding student assignments but there are many other potential use cases including package testing (for packages with plots)!
https://matplotcheck.readthedocs.io
BSD 3-Clause "New" or "Revised" License
18 stars 8 forks source link

Base Test Coverage and Test Organization #112

Closed marty-larocque closed 5 years ago

marty-larocque commented 5 years ago

This pull request does a number of things.

codecov[bot] commented 5 years ago

Codecov Report

Merging #112 into master will increase coverage by 1.84%. The diff coverage is 85.39%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #112      +/-   ##
==========================================
+ Coverage   58.04%   59.89%   +1.84%     
==========================================
  Files          14       16       +2     
  Lines        1230     1279      +49     
==========================================
+ Hits          714      766      +52     
+ Misses        516      513       -3
Impacted Files Coverage Δ
matplotcheck/tests/test_base_data.py 100% <100%> (ø)
matplotcheck/tests/test_base_titles_captions.py 100% <100%> (ø) :arrow_up:
matplotcheck/tests/test_base.py 100% <100%> (+4.76%) :arrow_up:
matplotcheck/tests/test_ts_data.py 100% <100%> (ø)
matplotcheck/tests/test_base_axis.py 100% <100%> (+1.63%) :arrow_up:
matplotcheck/tests/conftest.py 75.38% <26.66%> (-14.62%) :arrow_down:
matplotcheck/base.py 83.47% <88.23%> (+4.23%) :arrow_up:

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 f318e77...1d091a0. Read the comment docs.

marty-larocque commented 5 years ago

@lwasser Could you take a look at this?

marty-larocque commented 5 years ago

@lwasser The for loop and .iloc[] were temporary things I'd put in for debugging. I'm pretty confused how they ended up in the most recent commit, as I deleted them a couple commits ago, but they should be gone now.

lwasser commented 5 years ago

ok @ryla5068 this all looks great. i ddi some testing of black locally. for some reason it's not formatting the docstrings using the 79 character line length. i'm not sure why but let's just ensure the code is 79 characters is less long including the docstrings. i'll create an issue surrounding this but you can adjust the code manually for this pr. once you are done with the fixes, please ping me here again and i'll give this another review (and will likely merge as my comments are very small!!)

lwasser commented 5 years ago

a few other question

marty-larocque commented 5 years ago

@lwasser I think I've fixed everything here.

lwasser commented 5 years ago

excellent work on this PR @ryla5068 !!! i'm going to let CI run and will then merge this.