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

Python image piping not working on MSYS2 #2960

Open
Lestropie opened this issue Aug 12, 2024 · 7 comments
Open

Python image piping not working on MSYS2 #2960

Lestropie opened this issue Aug 12, 2024 · 7 comments

Comments

@Lestropie
Copy link
Member

Issue with #2678.

When constructing the location of a piped image, on Windows the current working directory is used (assuming TmpFileDir is not set), given that /tmp/ does not exist. This currently yields a Posix-style path.

However if the output of a Python script is piped to a C++ binary, the latter fails to locate that file, and the piped image remains on the file system. The C++ binary can be subsequently run standalone, copy-pasting that absolute path as input, and that works (presumably because the terminal emulator / shell is doing a path translation that is absent when that path is communicated via a pipe). Curiously, piping from a Python command to another Python command works.

  • Using an absolute path for piped images is a requirement: just specifying a file name will for most scripts result in the piped image file residing inside the scratch directory, which both by default gets deleted upon script completion and would regardless be in the wrong filesystem location for the command at the receiving end of the pipe to then resolve.

  • It is not a Windows line end encoding problem.

  • pathlib does not appear to have the requisite conversion capabilities.

If to resolve completely is going to require translation of paths to Windows format prior to sending down the pipe, I found this example, which invokes cygwin functionality via shared DLL; otherwise it could be custom code.
Also would need to fill app._STDOUT_IMAGES with instances of a custom class that uses the standard Posix path f-string formatting when invoking a command to generate it but a different function to write the path to stdout at script completion.

Open to any better ideas.

@Lestropie
Copy link
Member Author

It can be remedied temporarily by setting TmpFileDir to a Windows-style path in an MRtrix config file. Use of the -config command-line option however does not work, as piped image paths are determined at command-line parsing time whereas the config option only has an effect after command-line parsing has completed (I can potentially fix this by deferring the determination of a filesystem path until the first time it is used).

@daljit46
Copy link
Member

When constructing the location of a piped image, on Windows the current working directory is used (assuming TmpFileDir is not set), given that /tmp/ does not exist. This currently yields a Posix-style path.

Under an MSYS2 shell, /tmp/ should exist no?

@Lestropie
Copy link
Member Author

Hmmm so it does. I think I've gotten a couple a couple of wires crossed in my reasoning / description. There's been logic in place for quite some time to use a different default location for temporary piped files between Unix and Windows; see 9979917 & #199, though I don't think that's exhaustive regarding prior discussion on the topic. I seem to recall there was stronger reasoning at the time about preserving use of the working directory on Windows.

Nevertheless that observation doesn't provides a solution to the issue at hand: passing a /tmp/mrtrix-tmp-*.mif piped image through a pipe doesn't work either, it's still a Posix path that can't be interpreted by the downstream C++ binary receiving it.

The image path passed down the pipe has to be absolute, as the invoked command that generates it and the command at the receiving end of the pipe have different working directories. So either:

  • The Python API needs to translate output piped image paths from Posix to Windows; or
  • The C++ API needs to translate input piped image paths from Posix to Windows.

While we could hypothetically not support this feature on Windows, and immediately report an error if the user attempts to create an output piped image from a Python command, there are Python scripts that now internally make use of Python command piping, so the source code for those commands would need to be reverted accordingly.

@daljit46
Copy link
Member

The C++ API needs to translate input piped image paths from Posix to Windows.

My expectation is that using std::filesystem::path to handle image paths should be enough to deal with this. I'm still working on a PR to deal with #2585.

@Lestropie
Copy link
Member Author

I had a little bit of a look at std::filesystem::path, and didn't find anything that looked like it was going to do that conversion simply. Given that hypothetically one could send the piped output from an MRtrix3 command to a non-MRtrix3 command, I tend to think that whatever's emitted to stdout should be natively interpretable, in which case mrtrix3.app._execute() should be doing the translation. I might give that a go.

@jdtournier
Copy link
Member

Feel free to ignore if I'm completely wide of the mark, but I had to grapple with MSYS2's internal path translation while sorting out a few issues with the build and configure scripts some time ago, which may be useful in this context. Take a look at #1176, and the use of cygpath to perform the translation. No idea whether it'll be any use here, but just in case...

@Lestropie
Copy link
Member Author

I've already landed on using subprocess to call cygpath 😀

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

No branches or pull requests

3 participants