Skip to content

Conversation

@PaulWessel
Copy link
Member

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.

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.
@PaulWessel
Copy link
Member Author

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?

  1. Not release 1.1.5 until we release 6.1.1 presumably soon (August)?
  2. Release 1.1.5 on my DCW site but state it requires 6.1.1 or later?

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.
@PaulWessel PaulWessel changed the title WIP Let plotting of DCW in coast be done by plot Let plotting of DCW in coast be done by plot Jul 22, 2020
@PaulWessel PaulWessel requested review from joa-quim and seisman July 22, 2020 00:34
@seisman
Copy link
Member

seisman commented Jul 22, 2020

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.

@PaulWessel
Copy link
Member Author

Yes, those are good ideas. I will implement the checks and make it 1.2.0.

@PaulWessel
Copy link
Member Author

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.

@PaulWessel
Copy link
Member Author

Need to merge this in so we can plot DCW, @joa-quim and @seisman .

@PaulWessel
Copy link
Member Author

FYI, this PR is DCW-version neutral. It just switches plotting from inside gmt_dcw.c to a call via psxy.

@PaulWessel
Copy link
Member Author

You can run this branch with DCW 2.0.0 and it works - the checking of version is the other branch...

@joa-quim
Copy link
Member

joa-quim commented Jul 22, 2020

Needs Label 6.1.1

@PaulWessel PaulWessel merged commit 52ccc90 into master Jul 22, 2020
@PaulWessel PaulWessel deleted the new-dcw-plotting branch July 22, 2020 19:21
@joa-quim joa-quim added the backport 6.1 Backport this PR to 6.1 branch label Jul 22, 2020
github-actions bot pushed a commit that referenced this pull request Jul 22, 2020
* 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
PaulWessel added a commit that referenced this pull request Jul 22, 2020
* 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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 6.1 Backport this PR to 6.1 branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants