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 "save_to_disk" flag in the Diagram class to control automatic diagram file saving #1017

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Cole-M132
Copy link

This pull request introduces a new feature to the Diagram class: the save_to_disk flag. This flag allows users to control whether the generated diagram should be saved to disk or not. By default, the diagram is still saved to disk, which preserves the current behavior. However, by setting save_to_disk=False, users can prevent the diagram from being written to a file.

Key Amendments:
save_to_disk Flag:

Added a new optional parameter save_to_disk to the Diagram class constructor (init method).
The save_to_disk flag controls whether the diagram is saved to disk. If set to False, the diagram will not be saved as a file.

Modified the Diagram class render() method
The render() method now checks the value of save_to_disk before saving the diagram. If save_to_disk is False, the diagram rendering process will be skipped, and the diagram will not be saved to disk.

Modified the exit() method
The exit() method, which is called when exiting the with Diagram(...) context, now checks the save_to_disk flag. If save_to_disk is True, the diagram is rendered and saved as usual. If save_to_disk is False, the rendering is skipped.

Why These Changes Are Needed:
Currently, the Diagram class always saves the generated diagram to disk and users do not have the option to easily disable this. This behavior can be unnecessary or undesirable in situations where users only need to work with the diagram in memory in a byte stream (e.g., for further processing, testing, or streaming). The addition of the save_to_disk flag provides users with more flexibility and control over how their diagrams are handled.

@stasfilin
Copy link

Hi there,
Maybe it's better to use different variables, because right now it's not logical. What I see from code - you use save_to_disk for saving and rendering.

If we want to change this logic, I propose to use different flags for these two operations

  • save_to_disk - for saving
  • rendering (or something better) - to render

With this we will not mix logic for 1 flag.

Cheers

self.dot.render(format=one_format, view=self.show, quiet=True)
else:
self.dot.render(format=self.outformat, view=self.show, quiet=True)
if self.save_to_disk: # Only save if save_to_disk is True

Choose a reason for hiding this comment

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

Like for me better to do

    if not self.save_to_disk:
        return

to avoid unnecessary nesting

Copy link
Author

Choose a reason for hiding this comment

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

Hiya

Updated it to remove save_to_disk check in the render function all together. Now only checked in the exit() function before render() is even called. Saves doubling up on flag verification and mixing logic for one flag.

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