Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Map.Overlay tests to Maps.ipynb on add-map-overlay-tests branch #500

Merged
merged 5 commits into from
Jun 20, 2022

Conversation

Londala
Copy link
Contributor

@Londala Londala commented Apr 24, 2021

[X] Wrote test for feature
[ ] Added changes to CHANGELOG.md
[ ] Bumped version number (delete if unneeded)

Changes proposed:
Added test cases to test the Map.overlay() function in the Maps.ipynb file. The overlay() function itself seems to have some bugs, as the test cases were written as described in the function comments, but when run, exceptions are raised.

@davidwagner
Copy link
Member

If there are some bugs, let's get this fixed. Would you be willing to submit an issue for each bug you discovered, explaining what the bug is?

@davidwagner
Copy link
Member

Note to self: I found this easier to review with nbdiff -OD ... (using nbdime)

@adnanhemani
Copy link
Member

@davidwagner I've made some changes to this and fixed some bugs as well, so I don't think I can review this anymore 😅 Please go ahead and review/merge if this looks good to you. I'm aware that GH Actions won't run on this PR - but locally when running the new tests, I've gone from 86% to 91% coverage on the maps.py file (no test failures). Please let me know if there's anything else that needs a look here! :)

@davidwagner davidwagner removed the request for review from adnanhemani June 20, 2022 09:32
@davidwagner davidwagner merged commit 67ff872 into data-8:master Jun 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants