Skip to content

Base functionality, sphinx and PyPi config#3

Open
AmeerHamoodi wants to merge 48 commits intomasterfrom
feat/new-estimators
Open

Base functionality, sphinx and PyPi config#3
AmeerHamoodi wants to merge 48 commits intomasterfrom
feat/new-estimators

Conversation

@AmeerHamoodi
Copy link
Copy Markdown
Owner

This PR includes all of the base EEGPhasePy functionality:

  • Autoregressive phase estimation
  • Educated temporal prediction phase estimation
  • Bayesian optimization of AR phase estimation parameters
  • Genetic optimization of AR phase estimation parameters
  • Accuracy calculation of target vs true phase
  • Mean and standard deviation for triggers
  • Trigger window waveform plots
  • Polar histogram to visualize accuracy of phase estimation performance

API reference is also included in this PR using the readthedocs template

Comment thread gen_op_test.ipynb Outdated
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

This file will be removed after I've created the examples for EEGPhasePy usage. Keeping it here for now for reference

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Sep 3, 2025

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment

Thanks for integrating Codecov - We've got you covered ☂️

Copy link
Copy Markdown
Collaborator

@christianbrodbeck christianbrodbeck left a comment

Choose a reason for hiding this comment

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

Added some generic comments. For more detailed review, could you fill in the sphinx API documentation page, and add some usage examples? Sphinx-gallery may work well.

Comment thread EEGPhasePy/estimators/estimator.py Outdated
Comment thread EEGPhasePy/estimators/estimator.py Outdated
Comment thread EEGPhasePy/utils/check.py Outdated
Comment thread EEGPhasePy/utils/check.py Outdated
Comment thread EEGPhasePy/utils/check.py Outdated
Comment thread EEGPhasePy/estimators/estimator.py Outdated
Comment thread EEGPhasePy/estimators/estimator.py Outdated
Comment thread EEGPhasePy/estimators/estimator.py Outdated
AmeerHamoodi and others added 7 commits September 7, 2025 20:45
Co-authored-by: Christian Brodbeck <christianmbrodbeck@gmail.com>
Co-authored-by: Christian Brodbeck <christianmbrodbeck@gmail.com>
Co-authored-by: Christian Brodbeck <christianmbrodbeck@gmail.com>
@AmeerHamoodi
Copy link
Copy Markdown
Owner Author

Just partial, but wanted to let you know my comments so far. Will continue when I get another chance.

Thanks @christianbrodbeck, I've updated the PR based on your comments

@christianbrodbeck christianbrodbeck self-requested a review December 8, 2025 15:05
@AmeerHamoodi AmeerHamoodi requested a review from mus1223 January 15, 2026 17:28
Copy link
Copy Markdown
Collaborator

@mus1223 mus1223 left a comment

Choose a reason for hiding this comment

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

Left comments for sections: EEGPhasePy installation, ETP-based phase estimation, and the Alpha phase estimation using ETP pages.
For installation, main points are describing prior packages needed, and ensuring latest version of eegphasepy installs with provided command.
For ETP-based phase estimation page, main points are ensuring operability with Python 3.10 (or disclaimer to upgrade), some formatting notes, and additional recommendations to add.
For Alpha phase estimation using ETP page, main points are to reduce redundancy of repeating same code blocks, and displaying the expected figures with interpretation.

Comment thread docs/source/index.rst Outdated
Comment thread docs/source/index.rst
Comment thread docs/source/usage/etp.rst
Comment thread docs/source/usage/etp.rst
Comment thread docs/source/usage/etp.rst
Comment thread docs/examples/etp-based-phase-estimation.py Outdated
Comment thread docs/examples/etp-based-phase-estimation.py Outdated
Comment thread docs/examples/etp-based-phase-estimation.py Outdated
Comment thread docs/examples/etp-based-phase-estimation.py Outdated
Comment thread docs/examples/etp-based-phase-estimation.py Outdated
@AmeerHamoodi AmeerHamoodi requested a review from mus1223 January 24, 2026 21:16
Copy link
Copy Markdown
Collaborator

@mus1223 mus1223 left a comment

Choose a reason for hiding this comment

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

Review done for the following pages:
Alpha phase estimation using ETP
Autoregressive phase estimation
Bayesian optimization of PHASTIMATE parameters
Genetic optimization of PHASTIMATE parameters
Alpha phase estimation using PHASTIMATE
Optimizing PHASTIMATE parameters

Some main points are adding further explanation/rationale when needed, adding the output figures/stats themselves on to the tutorial along with their explanation, some potential inaccuracies with the stats values printed in some examples, and other considerations.

Comment thread docs/source/usage/ar.rst Outdated
Comment thread docs/source/usage/ar.rst Outdated
Comment thread docs/source/usage/ar.rst Outdated
Comment thread docs/source/usage/ar.rst
Comment thread docs/source/usage/ar.rst Outdated
Comment thread docs/examples/ar-phase-estimation.py Outdated
Comment thread docs/examples/ar-phase-estimation.py Outdated
Comment thread docs/examples/etp-based-phase-estimation.py Outdated
Comment thread docs/examples/etp-based-phase-estimation.py Outdated
Comment thread docs/source/usage/optimization.rst
@mus1223 mus1223 self-requested a review January 30, 2026 01:39
Copy link
Copy Markdown
Collaborator

@mus1223 mus1223 left a comment

Choose a reason for hiding this comment

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

Completed review for the following pages:
Plotting
Phase estimation statistics
Polar histograms
API reference

Main comments including providing more explanation where needed, typos, and missing information (particularly on API reference page)

Comment thread docs/source/api.rst
Comment thread docs/source/api.rst
Comment thread docs/source/usage/plots.rst
Comment thread docs/source/usage/plots.rst Outdated
Comment thread docs/source/usage/stats.rst Outdated
Comment thread docs/source/usage/stats.rst Outdated
Comment thread docs/source/usage/stats.rst Outdated
Comment thread docs/examples/polar-histogram.py
Comment thread docs/examples/polar-histogram.py
@AmeerHamoodi AmeerHamoodi requested a review from mus1223 April 19, 2026 22:03
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.

4 participants