Skip to content

Conversation

ariva00
Copy link
Member

@ariva00 ariva00 commented Jul 17, 2025

This is an implementation for issue #89.
I added the save method to ShapePlotter as an abstract method and implemented it in the concrete classes.
I'm not entirely sure it should be an abstract method, would it make sense for a plotter to not have a method to save the plot in image format?
The name of the method save might be ambiguous, maybe we could use a different alternative.

@gviga
Copy link
Collaborator

gviga commented Jul 18, 2025

Hi @ariva00 ,
Thank you for contributing!
I think we could expect to have a save method also as an abstract method.
For the name, I'm wondering if in the future we could have other methods to save images, for example, I'm expecting these functions to save images as PNG, maybe we will want a save_html in the future, we can also think of something like

Plotter.save_as_image(...) for saving image
Plotter.save_as_html(...) for saving html

Plotter.save(.., type='html'/'png') for usability

What do you think?

@ariva00
Copy link
Member Author

ariva00 commented Jul 18, 2025

Maybe we could keep save as the abstract method (with an argument type or infer the type from the extension) and have each concrete plotter implement its specific save_as_type, that depend on the backend, alongside the save method.
This way each plotter would be able to expose all the functionality of the backend and we could still have a common interface that throws an exception if the requested type is not available

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants