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

PurePath screws up HDF5 table construction on Windows systems #2318

Closed
cvaneldik opened this issue Apr 25, 2023 · 2 comments · Fixed by #2319
Closed

PurePath screws up HDF5 table construction on Windows systems #2318

cvaneldik opened this issue Apr 25, 2023 · 2 comments · Fixed by #2319
Labels

Comments

@cvaneldik
Copy link

Describe the bug
On Windows systems, code such as

with HDF5TableWriter(some_filename) as writer:
    writer.write(table_name="test", containers=some_container)

crashes with error NameError: ``where`` must start with a slash ('/')

The reason is that HDF5TableWriter._setup_new_table() makes use of pathlib.PurePath to set up HDF5 table name and path. The HDF5 format expects slashes ("/") as a separator for the internal data organisation.

On Windows systems, however, PurePath automatically evaluates to PureWindowsPath which expands any slash to a back-slash ("\"), screwing up the internal HDF5 path description.

Proposed solution

The proposed solution is to replace PurePath by PurePosixPath in HDF5TableWriter._setup_new_table(), specifically in

table_path = PurePath(self._group) / PurePath(table_name)

(However, I didn't test this, as I don't have access to a Windows system myself).

To Reproduce
Minimal example for what happens in HDF5TableWriter._setup_new_table():

group = "/"
table_name = "example_table"

table_path = PurePath(group) / PurePath(table_name)
table_group = str(table_path.parent)
table_basename = table_path.stem
table_path = str(table_path)

print(table_path)
print(table_group)

This results on Windows systems in

\example_table
\

Expected behavior

For the HDF5 internal data structure, I would expect that the result should look as follows:

/example_table
/

This is what is produced on Linux systems.

Supporting information
If applicable, add screenshots, logs, or a traceback to help illustrate the bug.

Additional context
Add any other context about the problem here.

@cvaneldik cvaneldik added the bug label Apr 25, 2023
@maxnoe
Copy link
Member

maxnoe commented Apr 25, 2023

Hi Christopher,

Please note ctapipe in general does not support Windows. We have had some best effort PRs lately, but it's definitely a non-goal to officially support windows and ensure compatibility.

The recommended way for running ctapipe on windows would be to use WSL.

That being said, the issue here looks easy to fix and of course we probably shouldn't be using the filesystem APIs for hdf5 related stuff anyways.

@cvaneldik
Copy link
Author

Hi Max,

Thanks for the fast response. Yes, I suspected that Windows compatibility is not foreseen (and I undestand why)...
OK, I'll tell my students to use WSL then!

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

Successfully merging a pull request may close this issue.

2 participants