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

[Docs] ecg_process: improve documentation and readability #871

Merged
merged 2 commits into from
Aug 10, 2023

Conversation

jchromik
Copy link
Contributor

Description

This PR aims at updating and improving the documentation and code readability of the ecg_process() function.

Proposed Changes

This PR does not change any external behaviour of the function. Just refactoring and documentation.

Checklist

Here are some things to check before creating the PR. If you encounter any issues, do let us know :)

  • I have read the CONTRIBUTING file. (ISSUE/PLEASE NOTE: The section Structure and code says that "The maximum line length is 100 characters." but in the section Run code checks the line length is specified as 120 characters.)
  • My PR is targetted at the dev branch (and not towards the master branch).
  • I ran the CODE CHECKS on the files I added or modified and fixed the errors.
  • I have added the newly added features to News.rst (if applicable)

@welcome
Copy link

welcome bot commented Jul 25, 2023

Thanks for opening this pull request! We'll make sure it's perfect before merging 🤗 force
Make sure to read the contributing guide. Also, if you think that your contribution is worthy of it, you can consider adding yourself to the Contributors list (feel free to ask us if you have any doubts).

@DominiqueMakowski
Copy link
Member

Hi @jchromik sorry I completely missed that, we'll review shortly :) thanks a lot!

Copy link
Collaborator

@danibene danibene 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 this @jchromik ! I left some comments

neurokit2/ecg/ecg_process.py Show resolved Hide resolved
neurokit2/ecg/ecg_process.py Outdated Show resolved Hide resolved
neurokit2/ecg/ecg_process.py Outdated Show resolved Hide resolved
neurokit2/ecg/ecg_process.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@danibene danibene 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 very much @jchromik ! @DominiqueMakowski feel free to merge

@codecov-commenter
Copy link

codecov-commenter commented Aug 3, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.01% 🎉

Comparison is base (a802b57) 55.23% compared to head (bfbf012) 55.24%.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #871      +/-   ##
==========================================
+ Coverage   55.23%   55.24%   +0.01%     
==========================================
  Files         297      297              
  Lines       13917    13916       -1     
==========================================
+ Hits         7687     7688       +1     
+ Misses       6230     6228       -2     
Files Changed Coverage Δ
neurokit2/ecg/ecg_process.py 100.00% <100.00%> (ø)

... and 3 files with indirect coverage changes

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

@danibene danibene merged commit f870193 into neuropsychology:dev Aug 10, 2023
@welcome
Copy link

welcome bot commented Aug 10, 2023

landing
Congrats on merging your first pull request! 🎉🍾 We're looking forward to your next one!

@jchromik jchromik deleted the improve_docs_ecg_process branch December 13, 2023 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants