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 abstract base class for SignalModel #132

Merged
merged 2 commits into from
Apr 20, 2021
Merged

Conversation

nik-sm
Copy link
Member

@nik-sm nik-sm commented Apr 19, 2021

Overview

Added an abstract base class SignalModel in preparation for refactoring the current PCA/RDA/KDE model and then integrating a new neural network model.

Ticket

https://www.pivotaltracker.com/story/show/177691543

Contributions

bcipy/signal/model/model.py - Add abstract base class
bcipy/signal/model/__init__.py - Make abstract base class available using from bcipy.signal.model import SignalModel

Test

No tests added for this abstract class.

Documentation

The base class has simple descriptions of the purpose of each method. Once the PCA/RDA/KDE model is refactored to this structure, more documentation can be added if useful. Alternatively, a "dummy" example class could be added if that would be useful.

@tab-cmd tab-cmd changed the base branch from main to 1.5.1 April 19, 2021 17:52
@tab-cmd tab-cmd requested review from tab-cmd and lawhead April 19, 2021 17:53
Copy link
Contributor

@tab-cmd tab-cmd left a comment

Choose a reason for hiding this comment

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

Awesome to see this! Thanks, Niklas! 😄 Let's add some information about creating new models using the base class to the README.md. https://github.com/CAMBI-tech/BciPy/tree/main/bcipy/signal

bcipy/signal/model/model.py Outdated Show resolved Hide resolved
bcipy/signal/model/model.py Outdated Show resolved Hide resolved
bcipy/signal/model/model.py Outdated Show resolved Hide resolved
@nik-sm nik-sm merged commit d1fe824 into 1.5.1 Apr 20, 2021
@nik-sm nik-sm deleted the signal_model_abstract_class branch April 20, 2021 00:30
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.

3 participants