Skip to content
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

Wrap xyz2grd #636

Merged
Merged

Conversation

carocamargo
Copy link
Contributor

@carocamargo carocamargo commented Sep 29, 2020

This PR wraps the function, xyz2grd and some of its parameters.

Closes #635

@seisman seisman added the feature Brand new feature label Sep 29, 2020
@weiji14 weiji14 mentioned this pull request Oct 24, 2020
5 tasks
@weiji14 weiji14 force-pushed the newfeature_xyz2grd branch from 715136e to c6eba8a Compare October 24, 2020 09:47
@weiji14
Copy link
Member

weiji14 commented Oct 24, 2020

Sorry for the force-pushes, had to resolve a tricky merge conflict. If you run git switch newfeature_xyz2grd locally, and then git pull, this should bring in the resolved changes. It's getting late for me now, but I'll try and take a closer look at this next week.

@seisman seisman changed the title Newfeature xyz2grd wrapper Wrap xyz2grd Nov 4, 2020
@carocamargo
Copy link
Contributor Author

I'm sorry I disappeared, I kind that forgot about this :X .. Do I still need to push the changes, or not anymore?

@willschlitzer willschlitzer self-assigned this Jul 31, 2021
@willschlitzer
Copy link
Contributor

I removed some of the optional parameters to simplify this PR. I figure it's easier to get it merged and then add additional parameters in separate pull requests.

@maxrjones
Copy link
Member

I removed some of the optional parameters to simplify this PR. I figure it's easier to get it merged and then add additional parameters in separate pull requests.

Thanks for working on this! Sorry for the slowness in reviewing your PRs, they have been lower on my to-do list since we are at a feature pause until v0.4.1. I will take a look after the release.

@willschlitzer
Copy link
Contributor

Thanks for working on this! Sorry for the slowness in reviewing your PRs, they have been lower on my to-do list since we are at a feature pause until v0.4.1. I will take a look after the release.

No worries, I expected a v0.4.1, but looking forward to getting them wrapped up!

@willschlitzer willschlitzer marked this pull request as ready for review August 7, 2021 06:48
@willschlitzer willschlitzer requested a review from weiji14 August 8, 2021 06:29
Copy link
Member

@weiji14 weiji14 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some formatting stuff, otherwise good for final review!

pygmt/src/xyz2grd.py Outdated Show resolved Hide resolved
pygmt/src/xyz2grd.py Outdated Show resolved Hide resolved
pygmt/src/xyz2grd.py Outdated Show resolved Hide resolved
pygmt/src/xyz2grd.py Outdated Show resolved Hide resolved
@weiji14 weiji14 added the final review call This PR requires final review and approval from a second reviewer label Aug 8, 2021
Copy link
Member

@seisman seisman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

pygmt/src/xyz2grd.py Outdated Show resolved Hide resolved
@willschlitzer willschlitzer merged commit 097c807 into GenericMappingTools:main Aug 9, 2021
@willschlitzer willschlitzer removed the final review call This PR requires final review and approval from a second reviewer label Aug 9, 2021
sixy6e pushed a commit to sixy6e/pygmt that referenced this pull request Dec 21, 2022
*Wrap the GMT module xyz2grd
*Add tests for xyz2grd

Co-authored-by: Dongdong Tian <[email protected]>
Co-authored-by: Wei Ji <[email protected]>
Co-authored-by: Will Schlitzer <[email protected]>
Co-authored-by: Meghan Jones <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Brand new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

xyz2grd wrapper
5 participants