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

Export image #11

Merged
merged 20 commits into from
Feb 3, 2025
Merged

Export image #11

merged 20 commits into from
Feb 3, 2025

Conversation

nathancoliver
Copy link
Member

  • Add write_image_plot to save plots as images with option to set scale (image resolution).
  • Add width and height of image to DimensionalPlot (width and height could be moved to write_image_plot).
  • Add kaleido to pyproject.toml, a plotly requirement to save plots as images.

@nathancoliver nathancoliver added the enhancement New feature or request label Jan 21, 2025
@nathancoliver nathancoliver self-assigned this Jan 21, 2025
Comment on lines 230 to 231
width: int | None = None,
height: int | None = None,
Copy link
Member

Choose a reason for hiding this comment

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

It looks like these are never set in any calls to this function. Can we remove them?

Copy link
Member Author

Choose a reason for hiding this comment

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

width and height are set in lines 245 and 246. Is that what you mean, or is there something else wrong?

Copy link
Member

Choose a reason for hiding this comment

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

There are two calls to finalize_plot (line 461 and line 471), but neither of them actually pass in width or height so they are always None.

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you want width and height to be passed into finalize_plot? I was debating whether to pass them into finalize_plot or DimensionalPlot.

If passed into DimensionalPlot, then jpeg and html plots are both adjusted. If only passed into write_plot_image (as well as finalize_plot), then jpeg files are the only files with modified height and width, which makes more sense to me.

We could also pass in width and height into write_html_plot, but I don't think that is necessary.

Copy link
Member Author

Choose a reason for hiding this comment

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

Please review latest changes

Comment on lines 463 to 466
def write_image_plot(self, path: Path, scale: int | None = None) -> None:
"Write plots to html file at specified path."
self.finalize_plot()
self.figure.write_image(path, scale=scale)
Copy link
Member

Choose a reason for hiding this comment

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

It would be good to add a comment to explain what "scale" is and how it is used.

Copy link
Member Author

Choose a reason for hiding this comment

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

Take a look at the updated write_image_plot.

Copy link
Member

Choose a reason for hiding this comment

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

Looks like a formatter was automatically applied to this file, causing unrelated diffs. Not sure I like it.

Copy link
Member Author

Choose a reason for hiding this comment

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

What do you want to do about it?

Copy link
Member

Choose a reason for hiding this comment

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

Change your editor settings so it doesn't automatically format TOML files (or at least disable the reordering of keywords).

Copy link
Member Author

Choose a reason for hiding this comment

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

I can't find the setting. I will need your help.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

pyproject.toml Outdated Show resolved Hide resolved
Comment on lines 230 to 231
width: int | None = None,
height: int | None = None,
Copy link
Member

Choose a reason for hiding this comment

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

There are two calls to finalize_plot (line 461 and line 471), but neither of them actually pass in width or height so they are always None.

Copy link
Member

Choose a reason for hiding this comment

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

Change your editor settings so it doesn't automatically format TOML files (or at least disable the reordering of keywords).

@nealkruis nealkruis merged commit c0970f8 into main Feb 3, 2025
12 checks passed
@nealkruis nealkruis deleted the export-image branch February 3, 2025 18:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants