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

Adapt implementation from lace #1

Merged
merged 9 commits into from
Mar 3, 2020
Merged

Adapt implementation from lace #1

merged 9 commits into from
Mar 3, 2020

Conversation

paulmelnikow
Copy link
Member

@paulmelnikow paulmelnikow commented Feb 28, 2020

This is a refactor / rewrite of the mesh–plane intersection code in Lace:

https://github.com/lace/lace/blob/a19d14eeca37a66af442c5a73c3934987d2d7d9a/lace/intersection.py
https://github.com/lace/lace/blob/a19d14eeca37a66af442c5a73c3934987d2d7d9a/lace/test_intersection.py

I've filled in a couple gaps in test coverage, though there are still a few more.

The docs are getting built on Netlify.

@paulmelnikow
Copy link
Member Author

@jbeard4 Any thoughts on how to handle code coverage for this? I've removed some dead code though there are a handful of uncovered lines yet. We could add coverage ignores and open an issue, or else maybe it's worth my taking maybe half an hour to find a few more examples that exercise the whole codebase.

@jbeard4
Copy link

jbeard4 commented Mar 3, 2020

I like the approach of adding pragmas to skip coverage so that you are unblocked, and creating issues to add tests for these lines so that they do not get lost. I went ahead and added these pragmas in 1162bfe. I will create github issues now.

@jbeard4
Copy link

jbeard4 commented Mar 3, 2020

I created a github issue for this: #2

@paulmelnikow paulmelnikow merged commit 9984dd1 into master Mar 3, 2020
@paulmelnikow paulmelnikow deleted the initial branch March 3, 2020 17:23
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.

2 participants