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 option single_ob to HDF5Merger for merging chunks of the same ob #2436

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

maxnoe
Copy link
Member

@maxnoe maxnoe commented Oct 30, 2023

Needed for merging of observations that are stored in multiple chunks into one final file for the OB.

TODO:

  • Add test

@kosack
Copy link
Contributor

kosack commented Oct 30, 2023

Is an option really needed for this? I would think the default behavior should just be updated to not duplicate an OB if it already exists.

@maxnoe
Copy link
Member Author

maxnoe commented Oct 30, 2023

Is an option really needed for this? I would think the default behavior should just be updated to not duplicate an OB if it already exists.

That could be done, yes. Depends a bit on the use case, e.g. we might want to check for uniqueness of obs_id and error out if not single_ob to prevent people from merging the same file twice or merging two simulation files that are different but have the same run number

@maxnoe maxnoe force-pushed the merge_chunks_single_obs_id branch from af2c9e0 to 4c334bf Compare November 23, 2023 17:33
Copy link

codecov bot commented Nov 23, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (4a45dcd) 92.46% compared to head (4c334bf) 92.46%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2436   +/-   ##
=======================================
  Coverage   92.46%   92.46%           
=======================================
  Files         234      234           
  Lines       19868    19872    +4     
=======================================
+ Hits        18370    18374    +4     
  Misses       1498     1498           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@maxnoe maxnoe force-pushed the merge_chunks_single_obs_id branch 2 times, most recently from 6de645b to ac0bd8e Compare February 5, 2025 16:56

This comment has been minimized.

@maxnoe maxnoe force-pushed the merge_chunks_single_obs_id branch from ac0bd8e to 13ea0ff Compare February 5, 2025 17:24
Copy link

Failed

  • B Maintainability Rating on New Code (is worse than A)
  • 60.00% Coverage on New Code (is less than 80.00%)

Analysis Details

1 Issue

  • Bug 0 Bugs
  • Vulnerability 0 Vulnerabilities
  • Code Smell 1 Code Smell

Coverage and Duplications

  • Coverage 60.00% Coverage (94.10% Estimated after merge)
  • Duplications 0.00% Duplicated Code (0.70% Estimated after merge)

Project ID: cta-observatory_ctapipe_AY52EYhuvuGcMFidNyUs

View in SonarQube

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.

2 participants