Adding basic support for a DataHandle that reads json into a dict

LSSTDESC / rail_base

Base classes for RAIL

MIT License

0 stars 1 forks source link

Adding basic support for a DataHandle that reads json into a dict #81

Closed drewoldag closed 6 months ago

drewoldag commented 7 months ago

Problem & Solution Description (including issue #)

This is a basic implementation of a DataHandle that is meant to read in a json file as a dictionary entirely into memory, no chunking needed. But to adhere to the ceci standard, we want to leverage the DataStore to do this.

Code Quality

[ ] My code follows the code style of this project
[ ] I have written unit tests or justified all instances of #pragma: no cover; in the case of a bugfix, a new test that breaks as a result of the bug has been added
[ ] My code contains relevant comments and necessary documentation for future maintainers; the change is reflected in applicable demos/tutorials (with output cleared!) and added/updated docstrings use the NumPy docstring format
[ ] Any breaking changes, described above, are accompanied by backwards compatibility and deprecation warnings

codecov[bot] commented 7 months ago

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (fd13285) 97.17% compared to head (cbe94c3) 96.97%. Report is 5 commits behind head on main.

Files	Patch %	Lines
src/rail/core/data.py	60.00%	6 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #81 +/- ## ========================================== - Coverage 97.17% 96.97% -0.21% ========================================== Files 32 32 Lines 1736 1816 +80 ========================================== + Hits 1687 1761 +74 - Misses 49 55 +6 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

drewoldag commented 6 months ago

We did some more learning, and I believe that this work won't be necessary. We were able to use the standard Hdf5 data handle to read in dictionaries serialized as JSON from an HDF5 file.

So I'm tempted to close this PR without merging unless someone has identified a good reason to incorporate it.

eacharles commented 6 months ago

I think it is fine to close this.

eacharles commented 6 months ago

Oops, I meant to comment that is could be closed, not close it.

eacharles commented 6 months ago

Ok, close this as per above