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 ECG Data Pipeline Tool #53

Merged
merged 20 commits into from
Mar 21, 2024
Merged

Add ECG Data Pipeline Tool #53

merged 20 commits into from
Mar 21, 2024

Conversation

Vicbi
Copy link
Collaborator

@Vicbi Vicbi commented Mar 9, 2024

Add ECG Data Pipeline Tool

♻️ Current situation & Problem

This PR adds the ECG Data Pipeline tool into a separate folder in the root of the project repository. Currently, the project lacks a dedicated pipeline for processing, analyzing and visualising the ECG data. This addition aims to provide a streamlined way to handle ECG data with enhanced analysis and visualisation capabilities.

⚙️ Release Notes

  • Added ECG Data Pipeline folder containing the pipeline tool for processing, analyzing, and visualising ECG data.
  • The pipeline includes modules for data preparation, access to Firebase, utility functions, and data visualization, alongside an interactive Python notebook (ECGDataPipelineTemplate.ipynb) for ECG data review.
  • Migration Guide: No breaking changes introduced. Users can integrate the pipeline tool into their workflow by cloning the repository and following the setup instructions in the README.
!git clone https://github.com/StanfordBDHG/PediatricAppleWatchStudy.git
%cd PediatricAppleWatchStudy/ECGDataPipeline

📚 Documentation

The ECG Data Pipeline tool is fully documented. The README within the ECG Data Pipeline folder provides an overview, setup instructions, and usage examples for integration and utilization of the tool.

✅ Testing

A job to build and test the ECG Data Pipeline notebook was added into the existing workflow build-and-test. This includes setting up Python, NodeJS, Java, and LaTeX environments; installing necessary dependencies; and executing the notebook with Firebase emulators to validate the data pipeline's functionality. The notebook is then converted to a PDF document, which is uploaded as an artifact for review.

Code of Conduct & Contributing Guidelines

By submitting creating this pull request, you agree to follow our Code of Conduct and Contributing Guidelines:

Copy link
Member

@PSchmiedmayer PSchmiedmayer left a comment

Choose a reason for hiding this comment

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

@Vicbi I have not taken a closer look at the Python code as I trust you with that and we will have to remodel this for the shared python package anyways.

I had a few suggestions about the documentation and general structure that we could incorporate.

Let's keep this PR open for a bit until we synced back with Scott and Aydin after the next meeting and the next iteration of the tool.

.github/workflows/build-and-test.yml Outdated Show resolved Hide resolved
ECGDataPipelineTemplate/README.md Outdated Show resolved Hide resolved
@PSchmiedmayer
Copy link
Member

@Vicbi The latest commit should fix the Xcode Build issue 👍

Copy link

codecov bot commented Mar 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 28.17%. Comparing base (2e271bb) to head (9d17378).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main      #53   +/-   ##
=======================================
  Coverage   28.17%   28.17%           
=======================================
  Files          34       34           
  Lines        1232     1232           
=======================================
  Hits          347      347           
  Misses        885      885           

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2e271bb...9d17378. Read the comment docs.

Copy link
Collaborator Author

@Vicbi Vicbi left a comment

Choose a reason for hiding this comment

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

Thanks for taking care of the Xcode Build issue @PSchmiedmayer ! All other changes were made 👍

Copy link
Member

@PSchmiedmayer PSchmiedmayer left a comment

Choose a reason for hiding this comment

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

Thank you for the improvements; I had a few comments and elements that I observed in the README and other parts of the code that would be great to address before we merge the PR.

ECGDataPipelineTemplate/README.md Outdated Show resolved Hide resolved
ECGDataPipelineTemplate/README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
ECGDataPipelineTemplate/README.md Outdated Show resolved Hide resolved
@Vicbi Vicbi requested a review from PSchmiedmayer March 20, 2024 19:38
@PSchmiedmayer PSchmiedmayer merged commit 84a0ddd into main Mar 21, 2024
5 checks passed
@PSchmiedmayer PSchmiedmayer deleted the addECGDataPipeline branch March 21, 2024 23:38
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