Skip to content

write_image() doesn't work with pathlib.Path() objects #2753

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

Closed
sedoris opened this issue Sep 8, 2020 · 9 comments · Fixed by #2974
Closed

write_image() doesn't work with pathlib.Path() objects #2753

sedoris opened this issue Sep 8, 2020 · 9 comments · Fixed by #2974

Comments

@sedoris
Copy link

sedoris commented Sep 8, 2020

This code works as expected:

import plotly.express as px
fig = px.scatter(x=[1,2,3], y=[1,2,3])
# Works with a string
fig.write_image('test.png', format='png')

However, if you try to use a pathlib.Path object instead of a string, you get an exception:

import plotly.express as px
import pathlib
fig = px.scatter(x=[1,2,3], y=[1,2,3])
# Doesn't work with a Path
fig.write_image(pathlib.Path('test.png'), format='png')

---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-6-402d35877fa7> in <module>
      6 fig.write_image('test.png', format='png')
      7 # Doesn't work with a Path
----> 8 fig.write_image(pathlib.Path('test.png'), format='png')

/opt/conda/lib/python3.8/site-packages/plotly/basedatatypes.py in write_image(self, *args, **kwargs)
   3249         import plotly.io as pio
   3250 
-> 3251         return pio.write_image(self, *args, **kwargs)
   3252 
   3253     # Static helpers

/opt/conda/lib/python3.8/site-packages/plotly/io/_kaleido.py in write_image(fig, file, format, scale, width, height, validate, engine)
    258             f.write(img_data)
    259     else:
--> 260         file.write(img_data)
    261 
    262 

AttributeError: 'PosixPath' object has no attribute 'write'

This code works, though:

import plotly.express as px
import pathlib
fig = px.scatter(x=[1,2,3], y=[1,2,3])
fig.write_image(str(pathlib.Path('test.png')), format='png')

So, at a minimum this could be fixed by checking if file is an instance of pathlib.Path and if so, cast it as a str.

PS Thanks for all the hard work on this project, it's great watching it continually improve. This isn't a particularly important issue for me, just wanted to add it to the list of potential fixes for the next version in case others are also interested.

@nicolaskruchten
Copy link
Contributor

Thanks for the bug report and the attempted PR :)

I wonder if there's not a more generic solution than spot-checking for pathlib objects, like "if it's not writeable, try to stringify it" or something like that.

maresb pushed a commit to maresb/plotly.py that referenced this issue Dec 14, 2020
@maresb
Copy link
Contributor

maresb commented Mar 5, 2021

@nicolaskruchten, any chance of getting a review on my #2974 PR? Thanks!

@dtoniolo
Copy link

dtoniolo commented Apr 2, 2021

+1 on this issue, I've encountered it too. @maresb, maybe the best way of handling pathlib.Path objects would be to use os.fspath(), as described in PEP 519. This would make the code compatible with all objects that adhere to the os.pathlike protocol

@maresb
Copy link
Contributor

maresb commented Apr 2, 2021

Thanks @dtoniolo for the support and the suggestion, I was unaware of os.fspath().

The huge disadvantage I see with the fspath() approach is the assumption that the given destination actually represents a file system path. For example, the destination might be a BytesIO stream. (I often use this trick to avoid unnecessary disk IO with other libraries which don't provide an analog of plotly.io.to_image().) Also, the pathlib interface can be extended to represent network resources, see for example the s3path package.

I would really like write_image() to robustly write to the destination, whatever it is.

@maresb
Copy link
Contributor

maresb commented Apr 4, 2021

@dtoniolo your comment inspired me to add a test case for BytesIO, so thank you!

(Just to be clear, there aren't many cases where it's desirable to use BytesIO with Plotly since plotly.io.to_image() is a cleaner alternative, but I think it makes a nice test case to ensure that Plotly can handle a writable buffer which doesn't correspond to a filesystem path.)

@dtoniolo
Copy link

dtoniolo commented Apr 6, 2021

Thanks @dtoniolo for the support and the suggestion, I was unaware of os.fspath().

The huge disadvantage I see with the fspath() approach is the assumption that the given destination actually represents a file system path. For example, the destination might be a BytesIO stream. (I often use this trick to avoid unnecessary disk IO with other libraries which don't provide an analog of plotly.io.to_image().) Also, the pathlib interface can be extended to represent network resources, see for example the s3path package.

I would really like write_image() to robustly write to the destination, whatever it is.

I wasn't aware of these technicalities, thank you for sharing them. Then your solution definitely covers the broadest range possible. I hope it gets accepted soon

@pascal456
Copy link

I landed here because I observed the same issue with write_html(), just wanted to notice that.

@maresb
Copy link
Contributor

maresb commented Apr 12, 2021

@pascal456 thanks for the report! That's really useful to know because I should build a test case for that.

maresb pushed a commit to maresb/plotly.py that referenced this issue Apr 23, 2021
@nicolaskruchten
Copy link
Contributor

Support for this just came out with version 5.0 :)

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 a pull request may close this issue.

5 participants