Visualization of causal graphs

[lgautier]

Hi,

Thanks for the package. This is great. I am proposing the following change. Opening an issue about the aspects believed to

The current visualization is writing into a PNG file, with a fixed default name ("causal_model.png"). The following aspects can be annoying.

• the argument is a base filename to which the suffix ".png" will be added can lead to surprises.This is also making the use of system-generated temp file more cumbersome than necessary (the suffix would need to be cut so it can be added again).
• the use of a fixed default file name in the current directory is too easily unsafe.
  • An existing file of the same name can accidentally be overwritten without warning
  • The use of the default by several processes in the same working directory will result in that file being overwritten

The proposed change is making the default filename a system-generated temporary file (to address the overwriting concern), is not adding a suffix".png" to the file name (to address the need to need to have to manually add the extension in code that would manage the file created), and is returning the file name (since it is not known beforehand when not specified). The drawbacks of the change is that repeated calls with default (unspecified) file name may result in filling the temporary directory unless the files created are managed (that is deleted). Overall this is a permanent fix. A refactoring/redesign of the API considering the two use-cases a) plot in a file specified by the user, and b) render immediately in-memory (for example to plot in a Jupyter notebook) would seem nicer.

[ghost]

Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

:x: lgautier sign now
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[amit-sharma]

thanks @lgautier for this contribution. I like the idea. Added a few comments above.

[amit-sharma]

@lgautier Checking in to see if we can merge this. Can you have a look at the CLA above and sign if everything's okay?

[emrekiciman]

Hello @lgautier, @amit-sharma, is this PR still active?

@lgautier, FYI since DoWhy moved out of the Microsoft GitHub organization, we know longer require the contributor license agreement. We ask for a developer certificate of origin, which is perhaps easier. See https://github.com/apps/dco

[petergtz]

@emrekiciman Can we close this PR due to inactivity?

[emrekiciman]

Yes. Let's go ahead and close this due to inactivity.

[lgautier]

Hi, things have been busy on this end. I'll probably go back to this, but some time in the future. Fine to close the PR if letting it open is looking disorderly.