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

Remove DataBundle.tables; instead overwrite DataBundle.data and use new property mode #238

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

ludwiglierhammer
Copy link
Collaborator

@ludwiglierhammer ludwiglierhammer commented Feb 21, 2025

Current status

Write data to new property DataBundle.tables after calling function method DataBundle.map_model.
Object DataBundle then contains data, mask and tables.
Processing huge data could explode the memory.
In addition, having both DataBundel.data and DataBundle.tables could lead to some confusion.

Solution

  • function method DataBundle.map_model overwrites DataBundle.data
  • funciton method DataBundle.map_model deletes DataBundle.mask
  • optionally, DataBundle.map_model does not overwrite DataBundle.data and does not delete DataBundle.mask but returns mapped tables as a pd.DataFrame

Questions

@jtsiddons: What do you think about this idea?

@codecov-commenter
Copy link

codecov-commenter commented Feb 21, 2025

Codecov Report

Attention: Patch coverage is 79.27928% with 23 lines in your changes missing coverage. Please review.

Project coverage is 84.10%. Comparing base (87403d8) to head (dd251a5).
Report is 78 commits behind head on main.

Files with missing lines Patch % Lines
cdm_reader_mapper/core/databundle.py 69.04% 13 Missing ⚠️
cdm_reader_mapper/cdm_mapper/reader.py 80.95% 8 Missing ⚠️
cdm_reader_mapper/core/reader.py 90.90% 1 Missing ⚠️
cdm_reader_mapper/core/writer.py 88.88% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #238      +/-   ##
==========================================
- Coverage   84.23%   84.10%   -0.14%     
==========================================
  Files          76       78       +2     
  Lines        2817     2863      +46     
==========================================
+ Hits         2373     2408      +35     
- Misses        444      455      +11     
Flag Coverage Δ
unittests 84.10% <79.27%> (-0.14%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@ludwiglierhammer
Copy link
Collaborator Author

@jtsiddons: I implemented two general functions with parameter mode:

  • cdm_reader_mapper.read:
    • mode == "mdf": calls cdm_reader_mapper.read_mdf
    • mode == "data": calls cdm_reader_mapper.read_data
    • mode == "tables": calls cdm_reader_mapper.read_tables
  • cdm_reader_mapper.write:
    • mode == "data": calls cdm_reader_mapper.write_data
    • mode == "tables": calls cdm_reader_mapper.write_tables

@github-actions github-actions bot added the docs label Feb 25, 2025
@jtsiddons
Copy link
Contributor

@jtsiddons: I implemented two general functions with parameter mode:

* `cdm_reader_mapper.read`:
  
  * `mode` == "mdf": calls `cdm_reader_mapper.read_mdf`
  * `mode` == "data": calls `cdm_reader_mapper.read_data`
  * `mode` == "tables": calls `cdm_reader_mapper.read_tables`

* `cdm_reader_mapper.write`:
  
  * `mode` == "data":  calls `cdm_reader_mapper.write_data`
  * `mode` == "tables":  calls `cdm_reader_mapper.write_tables`

To be clear, read and write are wrapper functions that call the appropriate function based on the mode argument?

Will the read_mdf, etc functions still be accessible in the same way as before?

@ludwiglierhammer
Copy link
Collaborator Author

ludwiglierhammer commented Feb 25, 2025

@jtsiddons: I implemented two general functions with parameter mode:

* `cdm_reader_mapper.read`:
  
  * `mode` == "mdf": calls `cdm_reader_mapper.read_mdf`
  * `mode` == "data": calls `cdm_reader_mapper.read_data`
  * `mode` == "tables": calls `cdm_reader_mapper.read_tables`

* `cdm_reader_mapper.write`:
  
  * `mode` == "data":  calls `cdm_reader_mapper.write_data`
  * `mode` == "tables":  calls `cdm_reader_mapper.write_tables`

To be clear, read and write are wrapper functions that call the appropriate function based on the mode argument?

Will the read_mdf, etc functions still be accessible in the same way as before?

Exactly, read and write are just wrapper functions. The other functions will still be accessible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants