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

Add new ax feature #109

Closed
wants to merge 8 commits into from
Closed

Conversation

FloraSauerbronn
Copy link
Contributor

New ax feature for plot_transect function in plotting.py

gliderpy/plotting.py Outdated Show resolved Hide resolved
@ocefpaf
Copy link
Member

ocefpaf commented Jun 17, 2024

@FloraSauerbronn When creating multiple figures or, of the user already passed an inverted axis, the call to invert it will undo this. We need to check if the axis is inverted first, and then invert it if not. See https://github.com/pyoceans/python-ctd/blob/10883153b27d296ef038252fe8f8604c79cd3a49/ctd/plotting.py#L30-L33 for a pattern on how to achieve this.

@ocefpaf
Copy link
Member

ocefpaf commented Jun 17, 2024

@FloraSauerbronn we also need to write some tests for this functionality. We can probably write two to cover the 3 cases in the notebook above.

  • 1 is already covered
  • 2 could be a figsize check to see if that is different from the default
  • the last one, for the subplots, should probably the another image comparison check

@@ -85,6 +85,9 @@ def plot_transect(
cbar = fig.colorbar(cs, orientation="vertical", extend="both")
cbar.ax.set_ylabel(var)
ax.set_ylabel("pressure")

ax.set_ylim(ax.get_ylim()[0], 0)
Copy link
Member

Choose a reason for hiding this comment

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

👍🏽

Copy link
Member

@ocefpaf ocefpaf left a comment

Choose a reason for hiding this comment

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

@FloraSauerbronn this one looks good! Let's remove the comments that are describing what the code is doing and add some docstrings to these functions?

Docstrings can be simple descriptions of test.


@pytest.mark.mpl_image_compare(baseline_dir=root.joinpath("baseline/"))
def test_plot_track(glider_data):
# Generate the plot
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
# Generate the plot

@pytest.mark.mpl_image_compare(baseline_dir=root.joinpath("baseline/"))
def test_plot_track(glider_data):
# Generate the plot
fig, ax = plot_track(glider_data)
# Return the figure for pytest-mpl to compare
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
# Return the figure for pytest-mpl to compare


glider_grab.fetcher.dataset_id = "whoi_406-20160902T1700"
df = glider_grab.to_pandas()
def test_plot_transect(glider_data):
# Generate the plot
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
# Generate the plot

# Generate the plot
fig, ax = plot_transect(df, 'temperature')
fig, ax = plot_transect(glider_data, 'temperature')
# Return the figure for pytest-mpl to compare
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
# Return the figure for pytest-mpl to compare


@pytest.mark.mpl_image_compare(baseline_dir=root.joinpath("baseline/"))
def test_plot_transect_multiple_figures(glider_data):
# Generate the plot with multiple figures
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
# Generate the plot with multiple figures

# Generate the plot with multiple figures
fig, (ax0, ax1) = plt.subplots(figsize=(15, 9), nrows=2, sharex=True, sharey=True)
glider_data.plot_transect(var="temperature", ax=ax0)
glider_data.plot_transect(var="salinity", ax=ax1, cmap=cmcrameri.cm.davos)
# Return the figure for pytest-mpl to compare
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
# Return the figure for pytest-mpl to compare

# Return the figure for pytest-mpl to compare
return fig

def test_plot_transect_size(glider_data):
# Generate the plot with a specific size
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
# Generate the plot with a specific size

@ocefpaf
Copy link
Member

ocefpaf commented Jun 25, 2024

@FloraSauerbronn I believe that all the changes here are part of the merged #114, right?

@ocefpaf ocefpaf closed this Jun 25, 2024
@FloraSauerbronn
Copy link
Contributor Author

FloraSauerbronn commented Jun 25, 2024

@FloraSauerbronn I believe that all the changes here are part of the merged #114, right?

Yes, all the changes are in #114

@FloraSauerbronn FloraSauerbronn deleted the plot-track branch June 27, 2024 16:18
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