- 
                Notifications
    You must be signed in to change notification settings 
- Fork 396
Let plotting of DCW in coast be done by plot #3696
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
Conversation
Because of the few polygons that have holes, we need to assemple a dataset and pass that to psxy in order for the holes to be properly handed.
| So GMT master (and hence 6.1.1 and beyond) can read dcw-gmt-1.1.x (x up to 5 which is not yet released), while GMT 6.1.0 an earlier can only use dcw-gmt-1.1.x up to 4. While future code could add a check on the DCW version, released code cannot. So what are our options? 
 I think most distros grab the tarballs from us, so we will leave at 1.1.4 as far as tarball goes until 6.1.1 is out. BUt once it is out, I guess we will continue to leave 1.1.4 there until the cows come home anyway. Thoughts? I have not removed the WIP although I fixed my memory leak since ex34 (involving the IT polygon) is causing some issues I need to look at. | 
A lower-case p pattern is inverted so need to write out P to avoid double conversion.
| Although there are only small changes in DCW, but it's not fully backward-compatible. Perhaps we should release DCW 1.2.0, rather than 1.1.5? I think we should make the official DCW 1.2.0 (or 1.1.5) release after releasing 6.1.1, but before that, it would be better to have the tarballs uploaded to your own FTP, so that at least @joa-quim and me can test the new DCW data for a while before the official GMT 6.1.1 and DCW 1.2.0 release. Is it possible to add one more attribute (the minimum required GMT version) to the DCW netCDF file, so that gmt_dcw.c can check if the current DCW data is compatible with the current GMT version? It won't help old releases <=6.1.0, but it will avoid similar troubles when we make similar incompatible changes in the future. | 
| Yes, those are good ideas. I will implement the checks and make it 1.2.0. | 
| The PR #3703 will help in future releases. This PR has no effect on 1.1.4 and is compatible with it and the future 1.2.0. I think this PR can be merged now. Tested and San Marino shows up as a hole in Italy, as does Lesotho in South Africa. I can even see the tiny hole of the Vatican. | 
| FYI, this PR is DCW-version neutral. It just switches plotting from inside gmt_dcw.c to a call via psxy. | 
| You can run this branch with DCW 2.0.0 and it works - the checking of version is the other branch... | 
| Needs Label 6.1.1 | 
* Let plotting of DCW in coast be done by plot Because of the few polygons that have holes, we need to assemple a dataset and pass that to psxy in order for the holes to be properly handed. * Update gmt_dcw.c * Set upper case P so not converted twice A lower-case p pattern is inverted so need to write out P to avoid double conversion. * Update gmt_dcw.c
* Let plotting of DCW in coast be done by plot Because of the few polygons that have holes, we need to assemple a dataset and pass that to psxy in order for the holes to be properly handed. * Update gmt_dcw.c * Set upper case P so not converted twice A lower-case p pattern is inverted so need to write out P to avoid double conversion. * Update gmt_dcw.c Co-authored-by: Paul Wessel <[email protected]>
Because of the few polygons that have holes, we need to assemble a dataset and pass that to psxy in order for the holes to be properly handed.
It is WIP for now because I need to track down some memory leaks related to the dataset. That part of the code had many bugs (it was never exercised) so this is a good test. However, with DCW 1.1.5 it correctly renders the holes and works as before (ignoring holes) with DCW 1.1.4. However, gmt 6.1.0 and earlier cannot use DCW 1.1.5 since the NaN coordinates have changed to pass the hole flag. So we need to prevent users from installing 1.1.5 unless they have GMT 6.1.2 or higher.