Skip to content

Conversation

@tomvothecoder
Copy link
Collaborator

@tomvothecoder tomvothecoder commented Jun 12, 2025

Description

Related to #987.

In #984, I refactored plotting utilities to make them more maintainable by sharing logic across plotters.

During refactoring, I added a call to plt.close() in _save_main_plot() (here). This prematurely closes the active plot figure, which results in subsequent calls to the plt library to produce blank figures -- including subplots in PDF format.

This PR now calls plt.close(fig) after all plots have been saved. It also replaces plt.close() with plt.close(fig) to ensure that the correct figure is closed.

  • plt.close() without arguments closes the current figure, which may not be the one you just saved, especially if you have multiple figures open.
  • plt.close(fig) ensures you are closing the specific figure you created and saved, which is safer and more robust in scripts and libraries.

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules

If applicable:

  • New and existing unit tests pass with my changes (locally and CI/CD build)
  • I have added tests that prove my fix is effective or that my feature works
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have noted that this is a breaking change for a major release (fix or feature that would cause existing functionality to not work as expected)

Copy link
Collaborator Author

@tomvothecoder tomvothecoder left a comment

Choose a reason for hiding this comment

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

return time


def _get_time_freq_and_start_time(time: xr.DataArray) -> Tuple[int, int]:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fix mypy pre-commit error.


_save_plot_aerosol_aeronet(fig, parameter)

plt.close(fig)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Example of a change to call plt.close(fig) AFTER saving all possible plots.

Comment on lines -51 to -57
def _save_plot_scatter(fig: plt.Figure, parameter: EnsoDiagsParameter):
"""Save the scatter plot using the shared _save_single_subplot function."""
_save_main_plot(parameter)

# Save the single subplot using shared helper (panel_config=None for full figure)
if parameter.output_format_subplot:
_save_single_subplot(fig, parameter, 0, None, None)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Moved these kinds of functions at the bottom because they are the last called private function, makes it easier to read file.


logger.info(f"Plot saved in: {filepath}")

plt.close()
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This line is what causes subsequent subplots to be missing in the PDF.

@tomvothecoder tomvothecoder self-assigned this Jun 12, 2025
@tomvothecoder tomvothecoder added the bug Bug fix (will increment patch version) label Jun 12, 2025
@tomvothecoder tomvothecoder force-pushed the bug/987-pdf-no-plots branch from 3af5f5e to 40b04eb Compare June 12, 2025 22:10
Copy link
Contributor

@chengzhuzhang chengzhuzhang left a comment

Choose a reason for hiding this comment

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

I re-ran the tropical_subseasonal set, and now the pdfs look as expected.

@chengzhuzhang chengzhuzhang merged commit 0811305 into main Jun 12, 2025
6 checks passed
@chengzhuzhang chengzhuzhang deleted the bug/987-pdf-no-plots branch June 12, 2025 23:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Bug fix (will increment patch version)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants