search

davidcsterratt / RTriangle

Port of the Triangle Two-Dimensional Quality Mesh Generator and Delaunay Triangulator to R
https://cran.r-project.org/package=RTriangle
9 stars 4 forks source link

Get rid of PB NA warning and improve NA handling overall. #11

Open MilesMcBain opened 6 years ago

MilesMcBain commented 6 years ago

I was going to make a fix just for https://github.com/davidcsterratt/RTriangle/issues/10 but then I noticed some issue with the way NA is handled. If NA accidentally makes its way into a parameter, the whole thing will be wiped and replaced with a default. I think this is unexpected and it would be kinder to the user to stop and notify as with P.

I made some explicit checks for a length 1 NA instead of any(is.na(param)), which is more precisely the case you wish to handle.

If you are prepared to accept this pull, don't merge immediately, let me know and I'll add a few tests. :)

davidcsterratt commented 8 months ago

A long time has passed - apologies for the long delay replying and thank you for the PR. If you are still interested, I'd be happy to consider a pull request with tests.

mdsumner commented 8 months ago

I'll do that if Miles cannot

davidcsterratt commented 8 months ago

I'd be happy for either of you to do this - I'm guessing that with the elapsed time, this project might not be relevant for Miles any more?

MilesMcBain commented 8 months ago

You have my blessing @mdsumner. I must admit to having completely dropped all context relating to this.

mdsumner commented 8 months ago

I actually think this is covered by #9, I think we just happened to hit on the same problem at the same time.

Happy to follow up if there's more to do but I think our PRs give equivalent behaviour. I'll try to look at testing at some point.

mdsumner commented 8 months ago

ah, I see that this PR adds more checks - having a closer look