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

Update Python integration to PsPM UI #675

Merged
merged 23 commits into from
Apr 11, 2024
Merged

Update Python integration to PsPM UI #675

merged 23 commits into from
Apr 11, 2024

Conversation

teddychao
Copy link
Contributor

@teddychao teddychao commented Mar 26, 2024

Updated 8-Apr-2024

Summery

  • Add python path definitions to the GUI

Results

  • src/pspm_cfg/pspm_cfg_pp_heart_data.m
    • The UI was updated by adding the choice of using HeartPy.
  • src/pspm_cfg/pspm_cfg_python.m
    • This is a new UI function pspm_cfg_python. The function defines the path of python with automatic or manual mode. It can be used by different features (not just HeartPy). It can be called by other GUI items if Python packages are required.
  • src/pspm_cfg/pspm_cfg_run_pp_heart_data.m
    • The function was updated by adding the features HeartPy.
  • src/pspm_convert_ppg2hb.m, src/pspm_options.m
    • The function was updated by bringing some missing code that was unexpectedly deleted.

@teddychao teddychao self-assigned this Mar 26, 2024
@teddychao teddychao added the In Progress Currently being worked on label Mar 26, 2024
@teddychao teddychao changed the title Update pspm_cfg_pp_heart_data.m Update Python integration Mar 26, 2024
@teddychao teddychao marked this pull request as ready for review March 27, 2024 01:25
@teddychao teddychao added Completed & Waiting for Review Completed and waiting for review and removed In Progress Currently being worked on labels Mar 27, 2024
@teddychao
Copy link
Contributor Author

Hi @MadniAbdulWahab
I just update the GUI by allowing users to specify the path of python installation. Could you have a look and see if it works on your computer? Thanks
image

Copy link
Contributor

@dominikbach dominikbach left a comment

Choose a reason for hiding this comment

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

See email

@dominikbach dominikbach added Changes Requested Authors are requested to make changes and removed Completed & Waiting for Review Completed and waiting for review labels Mar 27, 2024
@teddychao teddychao added In Progress Currently being worked on and removed Changes Requested Authors are requested to make changes labels Apr 5, 2024
@teddychao
Copy link
Contributor Author

This pull request only focuses on adding HeartPy option to the GUI. pspm_convert_ppg2hb has not been changed in this pull request.

@teddychao teddychao added Completed & Waiting for Review Completed and waiting for review and removed In Progress Currently being worked on labels Apr 7, 2024
@teddychao teddychao changed the title Update Python integration Update Python integration to PsPM UI Apr 7, 2024
@teddychao teddychao removed the Completed & Waiting for Review Completed and waiting for review label Apr 8, 2024
@teddychao teddychao added the In Progress Currently being worked on label Apr 8, 2024
src/pspm_cfg/pspm_cfg_pp_heart_data.m Outdated Show resolved Hide resolved
src/pspm_convert_ppg2hb.m Outdated Show resolved Hide resolved
src/pspm_convert_ppg2hb.m Outdated Show resolved Hide resolved
@@ -81,96 +85,152 @@
%% Large spikes mode
%--------------------------------------------------------------------------
Copy link
Contributor

Choose a reason for hiding this comment

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

The header "Large spikes mode" is placed wrongly (different from PR #612) - it should precede the associated code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image Sorry I am just a little unsure. I just noticed in the screenshot above (in PR #613 ) `%% Large spikes mode` has been removed. Shall I remove it here too? I think in the version you reviewed, I just forgot to remove this line..

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, please remove this line here.

@teddychao
Copy link
Contributor Author

Updated UI
image

@teddychao teddychao added Completed & Waiting for Review Completed and waiting for review and removed In Progress Currently being worked on labels Apr 8, 2024
Copy link
Contributor

@dominikbach dominikbach left a comment

Choose a reason for hiding this comment

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

PR #612 also contained a change to the function pspm_check_python_modules.m. This is required to run line 113 in the current version of pspm_convert_ppg2hb with heartpy method. Please add this change to the PR.

@@ -81,96 +85,152 @@
%% Large spikes mode
%--------------------------------------------------------------------------
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, please remove this line here.

@dominikbach dominikbach added Changes Requested Authors are requested to make changes and removed Completed & Waiting for Review Completed and waiting for review labels Apr 9, 2024
add updates to check_python and check_python_modules
@teddychao teddychao added Completed & Waiting for Review Completed and waiting for review and removed Changes Requested Authors are requested to make changes labels Apr 10, 2024
@teddychao teddychao merged commit 561a256 into develop Apr 11, 2024
1 check passed
@teddychao teddychao deleted the Python-GUI branch April 11, 2024 18:42
@teddychao teddychao removed the Completed & Waiting for Review Completed and waiting for review label Apr 11, 2024
@teddychao teddychao added this to the v6.2 milestone Apr 11, 2024
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.

None yet

2 participants