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

Let DataLoader default setters overwrite (not append) + lgdo.Struct.int_dtype #493

Merged
merged 18 commits into from
May 18, 2023

Conversation

gracesong312
Copy link
Collaborator

@gracesong312 gracesong312 commented May 15, 2023

  1. Adds the key_dtype to the Struct attributes so that we can use non-strings as keys in Struct. Converts to/from string when writing to/from disk. Also returns the output of dl.load(merge_files=False) as a Struct now that ints can be used as keys.
  2. Changes the default behavior of DataLoader setter methods to overwrite existing parameters, but adds an append argument in case appending is the desired behavior.

Addresses #492 first point

@codecov
Copy link

codecov bot commented May 15, 2023

Codecov Report

Patch coverage: 81.81% and project coverage change: +0.04 🎉

Comparison is base (f4f1940) 48.42% compared to head (2cf4cc4) 48.47%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #493      +/-   ##
==========================================
+ Coverage   48.42%   48.47%   +0.04%     
==========================================
  Files         104      104              
  Lines       12461    12498      +37     
==========================================
+ Hits         6034     6058      +24     
- Misses       6427     6440      +13     
Impacted Files Coverage Δ
src/pygama/lgdo/lh5_store.py 86.04% <66.66%> (-0.20%) ⬇️
src/pygama/flow/data_loader.py 77.01% <83.33%> (+0.52%) ⬆️
src/pygama/lgdo/struct.py 100.00% <100.00%> (ø)
src/pygama/lgdo/table.py 92.50% <100.00%> (ø)

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@iguinn iguinn mentioned this pull request May 15, 2023
8 tasks
@gipert gipert added the flow High-level data management label May 16, 2023
src/pygama/lgdo/lh5_store.py Outdated Show resolved Hide resolved
src/pygama/flow/data_loader.py Show resolved Hide resolved
@gipert
Copy link
Member

gipert commented May 17, 2023

Could you add a test of the append=False functionality? Something that makes sure that a second call to a setter + loading does not fail (i.e. does not require a call to .reset())/

src/pygama/lgdo/struct.py Outdated Show resolved Hide resolved
src/pygama/lgdo/struct.py Outdated Show resolved Hide resolved
@gipert gipert changed the title DataLoader - Default setter behavior + Struct key_dtype Let DataLoader default setters overwrite (not append) + lgdo.Struct.int_dtype May 18, 2023
@gipert gipert merged commit 9083f69 into legend-exp:main May 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flow High-level data management
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants