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

Utility to load topography #150

Merged
merged 8 commits into from
Nov 26, 2021
Merged

Utility to load topography #150

merged 8 commits into from
Nov 26, 2021

Conversation

gmaze
Copy link
Member

@gmaze gmaze commented Nov 19, 2021

Being to able to easily load local topography will be very useful for plotting or QC methods.
This PR will provide a new method to do this.

New API:

    from argopy import TopoFetcher
    box = [-75, -45, 20, 30]
    ds = TopoFetcher(box).to_xarray()

This will help for euroargodev/terrain-following#1

@gmaze gmaze added enhancement New feature or request argo-traj About trajectory data labels Nov 19, 2021
@gmaze gmaze added this to the Go from alpha to beta milestone Nov 19, 2021
@ocefpaf
Copy link
Collaborator

ocefpaf commented Nov 19, 2021

I have a similar prototype for a stand alone package for this (but I was using etopo2 instead). Do you think it would be better to factor it out in its own pkd so other packages can depend on it?

The downside is a slower development pace and higher maintenance cost.

@gmaze
Copy link
Member Author

gmaze commented Nov 23, 2021

Do you think it would be better to factor it out in its own pkd so other packages can depend on it?

In theory, I would agree
In practice this would lead to another dependency to manage and sustain
So I would promote the idea of a simple function/method to be easily/freely copied/pasted from one library to the other

@gmaze gmaze requested a review from quai20 November 23, 2021 09:21
@ocefpaf
Copy link
Collaborator

ocefpaf commented Nov 23, 2021

To be honest I was on the fence and that is why I never published my version :-)

Sounds like a good plan!

@gmaze gmaze removed the request for review from quai20 November 26, 2021 08:29
Copy link
Member Author

@gmaze gmaze left a comment

Choose a reason for hiding this comment

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

"I am @gmaze and I approve this message"

@gmaze gmaze merged commit 7ee9fba into master Nov 26, 2021
@gmaze gmaze deleted the topo branch November 26, 2021 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
argo-traj About trajectory data enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants