Base functionality, sphinx and PyPi config by AmeerHamoodi · Pull Request #3 · AmeerHamoodi/EEGPhasePy

AmeerHamoodi · 2025-09-02T23:55:25Z

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

…ests

AmeerHamoodi · 2025-09-02T23:59:40Z

gen_op_test.ipynb

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

codecov-commenter · 2025-09-03T12:40:35Z

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 ☂️

christianbrodbeck

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.

EEGPhasePy/estimators/estimator.py

EEGPhasePy/utils/check.py

EEGPhasePy/estimators/estimator.py

AmeerHamoodi · 2025-09-04T11:31:47Z

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.

I've completed the full documentation + examples and applied the generic code fixes

christianbrodbeck

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

docs/source/index.rst

docs/source/usage/etp.rst

docs/source/examples/etp-based-phase-estimation.ipynb

Co-authored-by: Christian Brodbeck <christianmbrodbeck@gmail.com>

…ePy into feat/new-estimators

AmeerHamoodi · 2025-09-14T16:01:25Z

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

mus1223

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.

mus1223 · 2026-01-22T22:12:11Z

docs/source/index.rst

+
+Getting started
+----------------
+To get started with EEGPhasePy, you will first need to install the packaeg from PyPi by running the command:


Adding a note that scipy, matplotlib packages are required would help

Added a requirements.txt file to auto-install these packages

mus1223 · 2026-01-22T22:13:19Z

docs/source/index.rst

+.. code-block:: Python
+    :caption: Install EEGPhasePy
+
+    pip install eegphasepy


this prompt automatically installs version 0.02. Suggest revising to the following to explicitly download latest version:
pip install --index-url https://test.pypi.org/simple/ --extra-index-url https://pypi.org/simple eegphasepy==0.0.4

I think this is related to the fact that I skipped version 0.0.3. When publishing on public pypi will ensure continuity in version numbers

mus1223 · 2026-01-22T22:17:18Z

docs/source/usage/etp.rst

+   import numpy as np
+   import scipy.signal as signal
+
+   from EEGPhasePy.estimators import ETP


This could not be recognized in Python 3.10.
Using Python 3.11 allowed this to work.

Removed typing issue that caused this issue in 3.10 and below. EEGPhasePy should be compatible with Python 3.9 and above. Thinking of also adding a job for each major Python version supported. Looks like MNE does something similar https://github.com/mne-tools/mne-python/blob/main/.github/workflows/tests.yml, will update our GH actions to do this as well

mus1223 · 2026-01-22T22:19:15Z

docs/source/usage/etp.rst

+   while window_start + window_len < len(testing_signal):
+       window = testing_signal[window_start:window_start + window_len]
+
+       if window_start + window_len + etp.predict(window, 0) < len(testing_signal) - window_len:


In Python 3.10, etp.predict(window, 0) gave the error: raise TypeError: Value must be a float type. Fix was to change to etp.predict(window, 0.0)
In Python 3.11 however, this error did not occur

Updated to allow both float and int

mus1223 · 2026-01-22T22:20:34Z

docs/source/usage/etp.rst

+       window_start += window_step
+
+   polar_hist_fig = etp.polar_histogram_from_triggers(testing_signal, triggers)
+   waveform_fig = etp.plot_mean_std_waveform_from_triggers(testing_signal, triggers, tmin=0.1, tmax=0.1)


It would be helpful to display the output figures on the tutorial itself, followed by a brief interpretation of each figure for the reader to understand)

mus1223 · 2026-01-23T00:48:22Z

docs/examples/etp-based-phase-estimation.py

+#
+# *A note on window length and edge effects:*
+#
+# When looking at other frequency bands that have more phase instability (as is the case with theta) or in cases of lower SNR, such as extracting phase data from the leisoned cortex of stroke patients, adjusting the window length and edge removed can significantly improve ETP performance. By increasing window length, more data is included in the sliding window allowing for a higher order filter to be used. This will help to more effectively extract the "signal" from "noise". However, increasing filter order will also increase the amount of data that is effected by filtering. Thus, you will need to increase the amount of signal removed from the edge of the window to compensate. This can potentially compromise ETP performance. It is important to balance all 3 of these factors


affected* instead of effected

mus1223 · 2026-01-23T00:57:04Z

docs/examples/etp-based-phase-estimation.py

+
+    if window_i + window_len + etp.predict(window, 0) < len(testing_signal) - 2*window_len:
+        triggers.append(window_i + window_len +
+                        etp.predict(window, (3/2)*np.pi))


It may be helpful to put a comment on this line, along the lines of: # change the target phase to your desired value here

mus1223 · 2026-01-23T01:01:19Z

docs/examples/etp-based-phase-estimation.py

+print("Accuracy: " + str(100*accuracy))
+
+# %% [markdown]
+# Similarly, to target troughs, instead of peaks, we just change the `target_phase` to :math:`\pi`


Repeating this whole block seems redundant, when the only change being made is to one line to adjust target phase. Would it be better to just list the one line where the change is made? Perhaps this whole chunk of code being unchanged can just be displayed once on this page, and subsequent changes to the triggers.append(window_i + window_len + etp.predict(window, np.pi)) line can just be displayed.

mus1223 · 2026-01-23T01:02:54Z

docs/examples/etp-based-phase-estimation.py

+print("Accuracy: " + str(100*accuracy))
+
+# %% [markdown]
+# Above, we estimated peaks using ETP. However, with ETP you can easily estimate any other phase by changing the `target_phase` argument to the desired phase in radians. Below we estimate alpha rising phase by simply changing `target_phase` to :math:`3\pi/2`


Same comment as below. Repeating this whole block of code seems redundant and can be viewed as a unnecessary burden for the reader. Better approach may be to just show the one line where the change is made, and list the rest of this needed code once above.

mus1223 · 2026-01-23T02:26:24Z

docs/examples/etp-based-phase-estimation.py

+        triggers.append(window_i + window_len + etp.predict(window, np.pi))
+    window_i += window_step
+
+polar_hist_fig = etp.polar_histogram_from_triggers(testing_signal, triggers)


It would be helpful for the output figures to be displayed, and a brief interpretation offered for each.

mus1223

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.

mus1223 · 2026-01-24T21:51:39Z

docs/source/usage/ar.rst

+filter edge effects and tens of milliseconds ahead of the current point in time, to account for Hilbert transform edge effects. AR was first introduced
+by :cite:t:`Zrenner2018-hh` and a toolbox for AR phase estimation known as PHASTIMATE was published by :cite:t:`Zrenner2020-zb`
+
+AR is best used in environments that make gaurentees about communication delays as foreward forecasting beyond the current point in time significantly reduces 


forward*
guarantees*

mus1223 · 2026-01-24T21:51:54Z

docs/source/usage/ar.rst

+
+AR is best used in environments that make gaurentees about communication delays as foreward forecasting beyond the current point in time significantly reduces 
+AR phase estimation performance. Further, to the best of our knowledge, AR has not been applied as a phase estimation algorithm in environments
+where delay gaurentees were made. It is also important to note that AR requires a high packet send rate from your EEG amplifier. This is due to the 


guarantees*

mus1223 · 2026-01-24T22:06:44Z

docs/source/usage/ar.rst

+   time_data = np.arange(0, 200, 1/fs)
+
+   signal_clean = np.sin(2 * np.pi * 10 * time_data)
+   signal_noisy = training_signal_clean + np.random.normal(0, 2, len(time_data))


the 'training_signal_clean' should just be 'signal_clean'

mus1223 · 2026-01-24T22:08:30Z

docs/source/usage/ar.rst

+        window_start += window_step
+
+    polar_hist_fig = phastimate.polar_histogram_from_triggers(signal_noisy, triggers)
+    waveform_fig = phastimate.plot_mean_std_waveform_from_triggers(signal_noisy, triggers, tmin=0.1, tmax=0.1)


Displaying both output figures on the tutorial page, along with an explanation/interpretation of the figures would help.

mus1223 · 2026-01-24T22:24:19Z

docs/source/usage/ar.rst

+
+AR is best used in environments that make gaurentees about communication delays as foreward forecasting beyond the current point in time significantly reduces 
+AR phase estimation performance. Further, to the best of our knowledge, AR has not been applied as a phase estimation algorithm in environments
+where delay gaurentees were made. It is also important to note that AR requires a high packet send rate from your EEG amplifier. This is due to the 


guarantees*

mus1223 · 2026-01-25T15:37:32Z

docs/examples/ar-phase-estimation.py

+    signal_noisy, triggers, degree=True)
+accuracy = phastimate.phase_accuracy_from_triggers(signal_noisy, triggers, 270)
+
+print("Mean phase: " + str(phase_mean))


My output:
Mean phase: 1.5927433244327647 --> is this in degrees? if so, shouldn't it be closer to 270?
Std phase: 1.8415718460550798
Accuracy: 99.527849327296

mus1223 · 2026-01-25T15:41:13Z

docs/examples/ar-phase-estimation.py

+    signal_noisy, triggers, degree=True)
+accuracy = phastimate.phase_accuracy_from_triggers(signal_noisy, triggers, 180)
+
+print("Mean phase: " + str(phase_mean))


My output:
Mean phase: 5.507510749476268 --> Is this in degrees? Shouldn't it be closer to 180?
Std phase: 2.3668169640597494
Accuracy: 99.85186634671345

mus1223 · 2026-01-25T15:45:58Z

docs/examples/etp-based-phase-estimation.py

+    testing_signal, triggers, degree=True)
+accuracy = etp.phase_accuracy_from_triggers(testing_signal, triggers, 270)
+
+print("Mean phase: " + str(phase_mean))


My output:
Mean phase: 2.5134114207524596 --> is this in radians? if so, shouldn't the numeric value be closer to 3pi/2?
Std phase: 3.266817681123747
Accuracy: 99.9852204969867

mus1223 · 2026-01-25T15:46:39Z

docs/examples/etp-based-phase-estimation.py

+    testing_signal, triggers, degree=True)
+accuracy = etp.phase_accuracy_from_triggers(testing_signal, triggers, 180)
+
+print("Mean phase: " + str(phase_mean))


My output:
Mean phase: 1.0619444569613414 --> is this in radians? If so, shouldn't the numeric value be close to pi?
Std phase: 2.707560758360248
Accuracy: 99.98973798577927

mus1223 · 2026-01-25T18:53:51Z

docs/source/usage/optimization.rst

@@ -0,0 +1,14 @@
+Optimizing PHASTIMATE parameters


I'd suggest just making this a subpage under 'Autoregressive phase estimation' for better organization.

mus1223

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)

mus1223 · 2026-01-30T01:45:51Z

docs/source/api.rst

+
+Here you can find the API reference for EEGPhasePy.
+
+Estimators


There is nothing displayed below this heading

mus1223 · 2026-01-30T01:46:01Z

docs/source/api.rst

+    :inherited-members:
+
+
+Visualization


There is nothing displayed below this heading

mus1223 · 2026-01-30T02:07:24Z

docs/source/usage/plots.rst

+.. code-block:: Python
+   :caption: Plotting polar histogram
+
+    polar_hist_fig = etp.polar_histogram_from_triggers(testing_signal, triggers)


Perhaps further instructions on modifying the look of the figure (if the user desires) can be helpful (e.g. colour, range of numbers to be displayed)

mus1223 · 2026-01-30T02:19:02Z

docs/source/usage/plots.rst

+------------------
+Plotting the average waveform with standard deivation highlights follows similar logic to the polar histogram. With the waveform average 
+however, you must specify how much time before and after the stimulus your plot should show. The time before stimulus delievery can be specified
+using ``tmin`` (always positive, larger values will encompass a greater amount of pre-stimulus time) and ``tmax``. You can 


You can say "(for e.g. tmin = 0.01 sec indicates 0.01 sec before t=0 sec)"

mus1223 · 2026-01-30T02:24:11Z

docs/source/usage/stats.rst

+
+Accuracy 
+---------
+Accuracy is defined as 50% begin completely random, 0% begin completely opposite to the target phase and 100% being exactly at the target phase. 


being* not begin

mus1223 · 2026-01-30T02:24:33Z

docs/source/usage/stats.rst

+Accuracy 
+---------
+Accuracy is defined as 50% begin completely random, 0% begin completely opposite to the target phase and 100% being exactly at the target phase. 
+We calculate accuracy based no the equation described by :cite:t:`Shirinpour2020-ef`. You can specify the target phase when you call the :py:meth:`EEGPhasePy.estimators.Estimator.phase_accuracy_from_triggers`


mus1223 · 2026-01-30T02:25:14Z

docs/source/usage/stats.rst

+.. code-block:: Python
+   :caption: Computing accuracy to target phase from triggers
+
+   accuracy = etp.phase_accuracy_from_triggers(testing_signal, triggers, 0)


Add a comment: "in this example, the target phase is 0"

mus1223 · 2026-01-30T02:28:04Z

docs/examples/polar-histogram.py

+fs = 2000
+time_data = np.arange(0, 200, 1/fs)
+
+signal_clean = np.sin(2 * np.pi * 10 * time_data)


add a brief sentence explaining this block of code

mus1223 · 2026-01-30T02:29:41Z

docs/examples/polar-histogram.py

+phase_data = np.angle(signal.hilbert(signal_clean))
+
+polar_hist = viz.plot_polar_histogram(
+    phase_data[peaks] + np.random.normal(0, 0.7, len(peaks)))


It would help to explain this line, such as: What is 0 and 0.7 representing here?

…e checking to account for float and int

AmeerHamoodi added 12 commits August 29, 2025 14:34

configured python package to work with pypi

1f80d31

initialized sphinx and added base documentation

bbb2bd9

added readthedocs yaml

c205524

refactored estimators to be based on parent class and completed etp t…

c0511a8

…ests

Updated API reference and sphinx theme

b6a698d

Added ETP plots, tests and updated docs

3296b83

auto-pep8 formatting added

644b90d

Added base AR phase estimator, test and docs

1f2ae5e

removed rough work

1dbee50

accounted for stochasticity of gaussian random noise in phastimate test

d2c3c0b

added bayesian optimization and genetic optimization for AR params

cc0314e

added requirements.txt for docs

fd0d32e

AmeerHamoodi requested a review from christianbrodbeck September 2, 2025 23:58

AmeerHamoodi commented Sep 2, 2025

View reviewed changes

AmeerHamoodi added 13 commits September 3, 2025 07:06

added gh workflow for tests and linting

92b09b3

modified action to only run on PR

d0a3528

specified pyproject path in actions

11a282c

fixing dependencies installation bug on actions

96f4016

fixed most linting errors

13396f2

fixed remaining lint errors and warnings

07b6491

fixed additional 2 linting errors

6308437

modified workflow to install pytest

6cba269

modified tests to only use mne on local

bee582a

removed unnecessary imports from tests

1d50ab2

run mne code on local only

f237f02

updated ci check in tests

baff476

added code-coverage reports

f06a966

christianbrodbeck requested changes Sep 3, 2025

View reviewed changes

minor code changes and full documentation push

0c3fa82

array conversion occurs outside of check

430690d

christianbrodbeck requested changes Sep 6, 2025

View reviewed changes

AmeerHamoodi and others added 7 commits September 7, 2025 20:45

Update docs/source/index.rst

a6f4065

Co-authored-by: Christian Brodbeck <christianmbrodbeck@gmail.com>

fixed typos, code blocks and added cross-refs

779f522

Update docs/source/usage/etp.rst

ff8a253

Co-authored-by: Christian Brodbeck <christianmbrodbeck@gmail.com>

Update docs/source/usage/etp.rst

0ee7fe3

Co-authored-by: Christian Brodbeck <christianmbrodbeck@gmail.com>

added wiki links

917cd90

Merge branch 'feat/new-estimators' of github.com:AmeerHamoodi/EEGPhas…

a391846

…ePy into feat/new-estimators

pip naming improvement and added sphinx-gallery

a695d42

christianbrodbeck self-requested a review December 8, 2025 15:05

AmeerHamoodi requested a review from mus1223 January 15, 2026 17:28

mus1223 requested changes Jan 23, 2026

View reviewed changes

AmeerHamoodi requested a review from mus1223 January 24, 2026 21:16

mus1223 reviewed Jan 25, 2026

View reviewed changes

mus1223 self-requested a review January 30, 2026 01:39

mus1223 reviewed Jan 30, 2026

View reviewed changes

fix: removed orphan from gallery, added requirements.txt, updated typ…

aa9ec3e

…e checking to account for float and int


		Here you can find the API reference for EEGPhasePy.

		Estimators

		:inherited-members:


		Visualization

Comments

Conversation

AmeerHamoodi commented Sep 2, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Welcome to Codecov 🎉

Uh oh!

christianbrodbeck left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AmeerHamoodi commented Sep 4, 2025

Uh oh!

christianbrodbeck left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AmeerHamoodi commented Sep 14, 2025

Uh oh!

mus1223 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mus1223 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Sep 3, 2025 •

edited

Loading