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

Invalid file name on Windows #17

Closed
drasmuss opened this issue Aug 19, 2019 · 3 comments · Fixed by #21
Closed

Invalid file name on Windows #17

drasmuss opened this issue Aug 19, 2019 · 3 comments · Fixed by #21

Comments

@drasmuss
Copy link
Member

To reproduce:

  1. Create file test.py
def test_plt(plt):
    plt.plot(range(100))
  1. Run pytest test.py --plots

Gives:

  File "d:\miniconda3\envs\nengo-dl\lib\site-packages\pytest_plt\plugin.py", line 188, in _finalize
    plotter.__exit__(None, None, None)
  File "d:\miniconda3\envs\nengo-dl\lib\site-packages\pytest_plt\plugin.py", line 144, in __exit__
    self.save(os.path.join(self.dirname, self.plt.saveas))
  File "d:\miniconda3\envs\nengo-dl\lib\site-packages\pytest_plt\plugin.py", line 152, in save
    self.plt.savefig(path, **savefig_kw)
  File "d:\miniconda3\envs\nengo-dl\lib\site-packages\matplotlib\pyplot.py", line 689, in savefig
    res = fig.savefig(*args, **kwargs)
  File "d:\miniconda3\envs\nengo-dl\lib\site-packages\matplotlib\figure.py", line 2094, in savefig
    self.canvas.print_figure(fname, **kwargs)
  File "d:\miniconda3\envs\nengo-dl\lib\site-packages\matplotlib\backend_bases.py", line 2075, in print_figure
    **kwargs)
  File "d:\miniconda3\envs\nengo-dl\lib\site-packages\matplotlib\backends\backend_pdf.py", line 2558, in print_pdf
    file = PdfFile(filename, metadata=metadata)
  File "d:\miniconda3\envs\nengo-dl\lib\site-packages\matplotlib\backends\backend_pdf.py", line 433, in __init__
    fh, opened = cbook.to_filehandle(filename, "wb", return_opened=True)
  File "d:\miniconda3\envs\nengo-dl\lib\site-packages\matplotlib\cbook\__init__.py", line 392, in to_filehandle
    fh = open(fname, flag, encoding=encoding)
OSError: [Errno 22] Invalid argument: 'plots\\tmp.test.py::test_plt.pdf'

I suspect it's the :: (since colons are used to denote drives on Windows).

@tbekolay
Copy link
Member

Apparently a single colon would be a problem as well, so we can't replace the double colon with a single colon. Probably best to replace colons with some other character, maybe a hyphen? tmp.test.py--test_plt.pdf seems fine to me.

The other question is whether we want to use this nodeid alternative consistently in other places, or consider it a test name specific thing.

@drasmuss
Copy link
Member Author

drasmuss commented Aug 19, 2019

Yeah just replacing : with something else seems fine to me. I'd probably keep using the standard nodeid elsewhere, since it's kind of a pytest standard. But in that vein, another question would be whether we want to keep using :: in the filenames for non-windows, or use -- everywhere?

Another option might be to put plots for each file in their own directory (basically replacing :: with /). That'd remove the ambiguity between the nodename and the file name (since it's clear they're related but distinct), but it does make the output directory less flat (I guess that could be a pro or a con).

@tbekolay
Copy link
Member

Another option might be to put plots for each file in their own directory (basically replacing :: with /).

Another downside would be an additional line or two to ensure that the directory is created before attempting to save the plot there. Right now we only have to do that once, for the general plot directory; this would require it for every plot.

Personally I prefer the flat output directory (even with the relatively large Nengo test suite, there are only 244 plots) but I can see why some would prefer more structure.

tbekolay added a commit that referenced this issue Oct 14, 2019
Windows does not allow colons in filenames. Rather than have
different behavior on different platforms, we unconditionally
replace colons with hyphens when generating the plot filename.
This is the only place that we modify the nodeid.

Fixes #17.
tbekolay added a commit that referenced this issue Oct 14, 2019
Windows does not allow colons in filenames. Rather than have
different behavior on different platforms, we unconditionally
replace colons with hyphens when generating the plot filename.
This is the only place that we modify the nodeid.

Fixes #17.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants