Conversation
|
I'm so confused why only macos and ubuntu 3.6 are failing... |
alessandrofelder
left a comment
There was a problem hiding this comment.
Looks good to me apart from some versioning issues for pyGMT and Python 3.6.
Seems that the ubuntu and mac CI install pygmt-0.2.1 which lacks some of the Clib functionality that we want to use?
Maybe worth trying to specify the minimal pygmt version required?
|
(Also at a later timepoint/if we have time, it might be cool to move this implementation into |
There was a problem hiding this comment.
Installing pygmt>=0.3.1 seems to have done the trick.
@JamieJQuinn can you update this in the README instructions on how to create an appropriate conda environment too, please, and then merge?
@Devaraj-G if you've got a pygmt version <0.3.1 in your cascadia conda environment (check this with conda list and then scroll down to p), the easiest solution is to remove your conda environment (conda env remove -n cascadia) and then re-create it following the instructions in the README, after this is merged (if you're using Python 3.9 you can ignore this).
|
@alessandro |
|
Perhaps could have been its own PR but I've added |
Significant changes have been made since last review
|
Phew! That could have gone better... @alessandrofelder take a look and reapprove if you're happy. |
alessandrofelder
left a comment
There was a problem hiding this comment.
Awesome! Love how nicely this simplifies the setup instructions.
This PR reimplements the nearneighbour call using pyGMT's clib instead of a manual os call to gmt. This is intended to help with the performance impact of writing temporary files (see #60). Although GMT will still write to a temporary file, pyGMT uses virtual files to ensure we don't have to write the input to GMT, so some savings should come from that. Additionally, we don't have to handle temporary files ourselves.