Skip to content

Vector vignette#220

Merged
lwasser merged 77 commits intoearthlab:masterfrom
nkorinek:vector-vignette
Apr 24, 2020
Merged

Vector vignette#220
lwasser merged 77 commits intoearthlab:masterfrom
nkorinek:vector-vignette

Conversation

@nkorinek
Copy link
Contributor

Vignette for how to test Vector objects with MatPlotCheck. WIP because I'm trying to troubleshoot one issue with checking points by markersize still.

@nkorinek
Copy link
Contributor Author

This PR requires that #203 is merged first!

@codecov
Copy link

codecov bot commented Mar 16, 2020

Codecov Report

Merging #220 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #220   +/-   ##
=======================================
  Coverage   79.95%   79.95%           
=======================================
  Files          19       19           
  Lines        1831     1831           
=======================================
  Hits         1464     1464           
  Misses        367      367           

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 da924a0...da924a0. Read the comment docs.

@jlpalomino
Copy link
Member

For our documentation, we have verbally discussed changes that will be completed by nkorinek, including the addition of more explanation about the object type that is being tested (e.g. a story about what kind of data is being used and what the test is actually looking for) both at the top of the notebook and in the comments, and a new section at the bottom of the vignette for additional options when using a Jupyter Notebook implementation.

@nkorinek
Copy link
Contributor Author

@jlpalomino ready for review!

Copy link
Member

@jlpalomino jlpalomino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few minor changes but otherwise looking good!

@lwasser lwasser added the documentation documentation for the package label Apr 20, 2020
@nkorinek
Copy link
Contributor Author

@jlpalomino sorry this took so long to get to, but I've added in the changes you requested!

@jlpalomino
Copy link
Member

@nkorinek No worries! And great work on this! Three super small modifications for the last changes.

After you make these small changes, feel free to ping lwasser to let her know that this PR is ready. Not sure if she wanted to take a look before merging. Thanks!

@nkorinek
Copy link
Contributor Author

@lwasser this is done btw!

@lwasser
Copy link

lwasser commented Apr 23, 2020

@jlpalomino you haven't approved this pr. have all of the changes been made to your satisfaction? if so please approve on your end and then i'll have a look!

@jlpalomino
Copy link
Member

@lwasser Sorry about that! Approval done. It's ready for your review.

Copy link

@lwasser lwasser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just letting ci run one more time and then i can merge!

@lwasser
Copy link

lwasser commented Apr 24, 2020

merging. thank you for your work on this pr @nkorinek @jlpalomino !!

@lwasser lwasser merged commit 38711b1 into earthlab:master Apr 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation documentation for the package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants