Skip to content

data_reader refactor#5

Open
MichalSzandar wants to merge 9 commits intomainfrom
data
Open

data_reader refactor#5
MichalSzandar wants to merge 9 commits intomainfrom
data

Conversation

@MichalSzandar
Copy link
Copy Markdown
Collaborator

@MichalSzandar MichalSzandar commented Mar 25, 2026

  • Create abstract class DataReader and use MatDataReader or CSVReader in main based on user's data file type

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors the data ingestion layer to introduce a DataReader abstraction and selects a reader in the CLI based on the input file type, while also parameterizing the analysis window length used for FFT computations.

Changes:

  • Introduce DataReader + MatDataReader scaffolding and update the pipeline entrypoint to use a reader instance.
  • Add CLI flags for --data_type and --output, and thread output_dir into FFT plotting calls.
  • Replace the hard-coded SAMPLES_PER_5_SEC usage in fft_analysis with WINDOW_TIME_FRAME.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 11 comments.

File Description
data/src/main.py Switches pipeline to use a DataReader instance and adds CLI args for reader selection and output directory.
data/src/data_reader.py Adds abstract DataReader and a MatDataReader implementation (plus a CSV stub).
data/src/consts.py Adds WINDOW_TIME_FRAME and a DATA_READERS mapping for CLI-driven reader selection.
data/src/fft_analysis.py Uses WINDOW_TIME_FRAME-derived sample counts for FFT frequency axis and magnitude scaling.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@kacper-daniel
Copy link
Copy Markdown
Collaborator

change CSVReader to FIFReader OR keep CSVReader and add FIFReader

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.

3 participants