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

Creating plotting.py with pandas-flavor #104

Merged
merged 7 commits into from
Jun 6, 2024

Conversation

FloraSauerbronn
Copy link
Contributor

@FloraSauerbronn FloraSauerbronn commented Jun 3, 2024

decorating the plot transect method.

gliderpy/plotting.py Outdated Show resolved Hide resolved

cbar = fig.colorbar(cs, orientation="vertical", extend="both")
cbar.ax.set_ylabel(var)
ax.set_ylabel("Depth (m)")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
ax.set_ylabel("Depth (m)")
ax.set_ylabel("pressure")

@ocefpaf
Copy link
Member

ocefpaf commented Jun 3, 2024

@FloraSauerbronn do you mind adding a description above, you can edit your empty box, so we have something for future reference? You can go as simple as "decorating the plot transect method."

BTW, one place where you oceanography background will com in handy, is in implementing some improvements to this function. For example, you can add an if-clause to error out if the variable chosen for plotting is not in an expected list of vars that would work. At the moment I believe that we can only do temperature and salinity.

@ocefpaf
Copy link
Member

ocefpaf commented Jun 3, 2024

We will need to import the plotting routine in the __init__.py to make it available as a DataFrame method. You can add something like:

from .plotting import plot_transect
from .fetchers import GliderDataFetcher


__all__ = [
    "GliderDataFetcher",
    "plot_transect",
]

to the __init__.py file. The GliderDataFetcher is not required for this PR but it is something we should've done a while back.

@FloraSauerbronn
Copy link
Contributor Author

FloraSauerbronn commented Jun 3, 2024

About the if clause @ocefpaf

--> When you say 'not in an expected list of vars,' do you mean they have a lot of NaNs? Or just outliers? Because if it's just NaNs causing the problem, it's simple, but if it's outliers, then we would need to consider the values of the limit more carefully.

@ocefpaf
Copy link
Member

ocefpaf commented Jun 3, 2024

About the if clause @ocefpaf

--> When you say 'not in an expected list of vars,' do you mean they have a lot of NaNs? Or just outliers? Because if it's just NaNs causing the problem, it's simple, but if it's outliers, then we would need to consider the values of the limit more carefully.

I mean the variables names. In the plot_transect we have one mandatory argument:

:param var: variable to colour the scatter plot

That can be either salinity or temperature, based on the list of variables we have are fetching:

server_vars = {
    "https://gliders.ioos.us/erddap": [
        "latitude",
        "longitude",
        "pressure",
        "profile_id",
        "salinity",
        "temperature",
        "time",
    ],
}

None of the others variables make sense to be in a section plot, right? However, let's keep this first PR simple and we can address that in a follow up one. reducing the context of the PRs will help us keep the momentum going and code changes/reviews easier to manage.

@ocefpaf ocefpaf mentioned this pull request Jun 4, 2024
@ocefpaf
Copy link
Member

ocefpaf commented Jun 6, 2024

pre-commit.ci autofix

@ocefpaf ocefpaf merged commit 2540a31 into ioos:main Jun 6, 2024
12 checks passed
@FloraSauerbronn FloraSauerbronn deleted the create-plotting branch June 19, 2024 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants