search

InsightSoftwareConsortium / ITK

Insight Toolkit (ITK) -- Official Repository. ITK builds on a proven, spatially-oriented architecture for processing, segmentation, and registration of scientific images in two, three, or more dimensions.
https://itk.org
Apache License 2.0
1.38k stars 661 forks source link

WIP: Use std filesystem path #4562

Open blowekamp opened 3 months ago

blowekamp commented 3 months ago

Add std::filesystem::path to ImageIOBase for eventual wchar support for paths.

This proposes an approach to remove direct access to m_FileName, with the eventual end goal to use GetFilePath and "legacy" GetFileName methods.

Add Set/Get FilePath member functions to ImageIOBase, with the future
plan to replace the m_FileName member variable with these access
member functions. The m_FileName is directly access by derived ImageIO
classes.

The m_FileName is now "constant" to disallow derived classes from
directly writing to the variable.  The SetFilePath method updates the
legacy const m_FileName to maintain compatibility.
N-Dekker commented 3 months ago

@blowekamp Very interesting, thanks! I definitely like having a separate type for file paths, rather than just "std::string", for improved type safety. I'm also curious to see if it does improve non-ASCII character support in file names, on Windows.

seanm commented 2 months ago

It looks like adopting std::filesystem would require increasing ITK's minimum macOS to 10.15:

https://stackoverflow.com/questions/58667853/does-use-of-c17-stdfilesystem-require-macos-10-15-xcode-11-1

Even though C++17 compilers exist on older version of macOS, the STL shipped on older versions does not support std::filesystem, from what I gather.

That would suck for me, I still support back to 10.13.

thewtex commented 2 months ago

I still support back to 10.13.

Our Python packages also require 10.11.