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

Add support to read from prefixed columns #1364

Merged
merged 3 commits into from
Jun 30, 2020

Conversation

LukasNickel
Copy link
Member

This fixes #1360 in order to work towards a DL1EventSource.

  • The HDF5TableReader.read() method now has an additional, optional argument prefix
  • Following the discussion in HDF5TableReader does not support prefixed columns #1360 three use cases are supported:
    • prefix=False: No prefix. This is the default, because per default the writer does not add a prefix as well
    • prefix=True: Using the container prefix
    • prefix="my_prefix": Using a custom prefix

This enables the reading of the image parameter containers, that are written by the stage1 tool, but support for reading into multiple containers at once is still missing.

@codecov
Copy link

codecov bot commented Jun 11, 2020

Codecov Report

Merging #1364 into master will increase coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1364      +/-   ##
==========================================
+ Coverage   90.88%   90.91%   +0.03%     
==========================================
  Files         184      184              
  Lines       12645    12676      +31     
==========================================
+ Hits        11492    11524      +32     
+ Misses       1153     1152       -1     
Impacted Files Coverage Δ
ctapipe/io/hdf5tableio.py 95.85% <100.00%> (+0.85%) ⬆️
ctapipe/io/tableio.py 90.90% <100.00%> (ø)
ctapipe/io/tests/test_hdf5.py 97.94% <100.00%> (+0.06%) ⬆️
ctapipe/image/extractor.py 86.53% <0.00%> (+0.53%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7798135...ab06822. Read the comment docs.

@LukasNickel
Copy link
Member Author

Does anyone know why codacy is still complaining?

@maxnoe
Copy link
Member

maxnoe commented Jun 12, 2020

Does anyone know why codacy is still complaining?

Yes, you changed the signature only of the hdf table reader, not of the base class. Methods should have the same signatures for all base classes.

@LukasNickel LukasNickel requested a review from kosack June 15, 2020 10:28
kosack
kosack previously approved these changes Jun 16, 2020
ctapipe/io/hdf5tableio.py Outdated Show resolved Hide resolved
ctapipe/io/hdf5tableio.py Outdated Show resolved Hide resolved
@LukasNickel
Copy link
Member Author

I am pretty sure codacy stopped complaining the last time I visited this.
Can we get this merged or is there anything left to do?

@LukasNickel LukasNickel mentioned this pull request Jun 19, 2020
@LukasNickel LukasNickel requested review from vuillaut and maxnoe and removed request for maxnoe June 26, 2020 09:24
@kosack kosack merged commit 689a731 into cta-observatory:master Jun 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HDF5TableReader does not support prefixed columns
3 participants