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

[WIP] Adding tests: base and raster #30

Closed kysolvik closed 5 years ago

kysolvik commented 5 years ago

Opening a PR as I work on adding some more tests.

Also, I was having issues with the precommit hook which used python3.6. I changed it to 3.7 to match the environment.yml file, but this could be undone. That's in commit 9d58835aabc576f568e11f93f14acdf9dc0408e2

codecov[bot] commented 5 years ago

Codecov Report

Merging #30 into master will increase coverage by 23.64%. The diff coverage is 97.69%.

@@             Coverage Diff             @@
##           master      #30       +/-   ##
===========================================
+ Coverage   13.73%   37.37%   +23.64%     
===========================================
  Files           8        9        +1     
  Lines         801      923      +122     
===========================================
+ Hits          110      345      +235     
+ Misses        691      578      -113

Impacted Files	Coverage Δ
matplotcheck/tests/test_raster.py	`96.29% <96.29%> (ø)`
matplotcheck/tests/test_base.py	`99.15% <98.68%> (+1.12%)`	:arrow_up:
matplotcheck/base.py	`47.65% <0%> (+24.21%)`	:arrow_up:
matplotcheck/raster.py	`86.88% <0%> (+86.88%)`	: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 5b4c5f6...b6d471a. Read the comment docs.

lwasser commented 5 years ago

thank you for this pr @kysolvik !!! please label your PR as a [WIP] work in progress so i know it's under development. when you are ready for a review, please remove WIP from the name and ping me to provide a review. I suggest that we organize sets of tests in separate branches. so perhaps this PR only covers test for base and raster.

Then we have a tests_vector etc for other tests. that way when you are done writing tests for a module (or a set of methods) the review process moves along more quickly. WHen PR's get too big they become challenging to review and accept.

kysolvik commented 5 years ago

hey @lwasser, sounds great! Sorry about that, thanks for the suggestion. I updated the name, I'll let you know when it's ready!

lwasser commented 5 years ago

oh please don't apologize @kysolvik we didn't talk about a workflow. but it's good if we work on one thing at a time just so i know what you are working on. Also i think appveyor is working so i'll likely merge that PR today or monday so we have windows testing too!! yahoo!

thank you again for this. it's great to have another set of eyes on this package!!

kysolvik commented 5 years ago

Closing, will split into multiple PRs