Skip to content

Implement the documented -N and update the docs#5247

Merged
PaulWessel merged 3 commits intomasterfrom
grdfill-fixing
May 27, 2021
Merged

Implement the documented -N and update the docs#5247
PaulWessel merged 3 commits intomasterfrom
grdfill-fixing

Conversation

@PaulWessel
Copy link
Copy Markdown
Member

We advertised -N but had not code it up; this PR does that. Also, we stated that -As had not been implemented but it has been for a long time. I have now improved the man page to mention all of this. Closes #5246 but will not be merged until after 6.2.0 has been released, hence WIP.

We advertised -N but did not code it up; this PR does that.  Also, we stated that -As had not been implemented but it has been for a long time. I have now improved the man page to mention all of this.
@PaulWessel PaulWessel added this to the Future release milestone May 22, 2021
@PaulWessel PaulWessel self-assigned this May 22, 2021
@maxrjones
Copy link
Copy Markdown
Member

The docs state that -A is required but grdfill without -A passes without warning (see below). Is -A required? If not, what is the default?

gmt grdclip @earth_relief_05m -R199:30/206/18/23 -Sa0/NaN -Gislands.nc
gmt grdfill islands.nc -Gnew.nc

@PaulWessel
Copy link
Copy Markdown
Member Author

I think -A should be required. It is not obvious that anyone would expect a default method in this module. So I think we should check for it and complain if not given. Again, the docs are clear while the grdfill synopsis prints it as optional. I will fix this herein.

@PaulWessel
Copy link
Copy Markdown
Member Author

I will do it in a separate PR.

@PaulWessel
Copy link
Copy Markdown
Member Author

Hm. So either -A or -L are required. If you use -L then -A is not required. I think when that happens in other modules we list both -A and -L as optional but state that one must be selected, right?

@maxrjones
Copy link
Copy Markdown
Member

Hm. So either -A or -L are required. If you use -L then -A is not required. I think when that happens in other modules we list both -A and -L as optional but state that one must be selected, right?

Yes and usually break with error if neither are provided.

@PaulWessel PaulWessel changed the title WIP Implement the documented -N and update the docs Implement the documented -N and update the docs May 27, 2021
@PaulWessel
Copy link
Copy Markdown
Member Author

Should be OK here now, @meghanrjones.

@PaulWessel PaulWessel modified the milestones: Future release, 6.2.0 May 27, 2021
@PaulWessel PaulWessel merged commit 671bd78 into master May 27, 2021
@PaulWessel PaulWessel deleted the grdfill-fixing branch May 27, 2021 00:49
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.

Is grdfill -N option available?

2 participants