-
Notifications
You must be signed in to change notification settings - Fork 272
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
OpticsDescription table is inconsistent (and not readable) #1359
Comments
see #1358 |
Are you going to add a second (newer) table to ctapipe-extra? Or what would be the next steps on this? |
This should be fixed by #1405, right? |
No I think it's a fundamental problem in the OpticsDescription to_table and from_table methods. They just need to be made consistent. There is a bit of a hack in #1405 to make it work, but we are definitely missing a unit test and having these two methods work without any hacks. The problem is that the file format changed in #982, but only the write method was updated, I think. |
Ok, so we also need to update a file in ctapipe-extra, right? |
Yes, or also support the old format (by looking at the TAB_VER header in the file). Might be a good example for what happens when we update the data model or file format in the future... |
Fixed by merging #1417 |
Writing an optics table with
subarray.to_table(kind="optics").write(output)
writes a table in a format that is not readable byOpticsDescription.from_name()
.It seems the format was updated in one place, but not the other. This leads to the files written by
ctapipe-stage1-process
to be not usable to construct a SubarrayDescription.With other tables, like CameraGeometry we were careful to set a
TAB_VER
(table version) header keywords, so we could support data model changes, but it seems that was not done here.Suggestion: we will have to add TAB_VER to this, but to support older version of the data format (which unfortunately are in ctapipe-extra and all output from ctapipe-stage1-process v0.8), we will have to make some code that reads both type (actually it seems 3 separate data models are assumed).
The text was updated successfully, but these errors were encountered: