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 mock neurodata objects #1454

Merged
merged 57 commits into from
Sep 16, 2022
Merged

add mock neurodata objects #1454

merged 57 commits into from
Sep 16, 2022

Conversation

bendichter
Copy link
Contributor

Motivation

fix #1452

I tried fixtures first but it was very difficult to get them to play nice with unitTests.

@bendichter bendichter requested review from rly and oruebel April 15, 2022 17:54
@bendichter
Copy link
Contributor Author

I'm happy with the flexibility of this approach. I think this could help minimize a l of testing code across several repos, including PyNWB, NWB Widgets, and NWB Inspector.

Questions:

  • How do people feel about this naming convention?
  • Should we try to automate creation from docval?

* add mock ophys types
* use new mock types in ophys unit tests
@bendichter bendichter self-assigned this Apr 19, 2022
* correct SpikeEventSeries timestamps
* move all mock tests do a dedicated testing file
# Conflicts:
#	tests/unit/test_ecephys.py
#	tests/unit/test_ophys.py
tests/unit/test_misc.py Outdated Show resolved Hide resolved
tests/unit/test_misc.py Outdated Show resolved Hide resolved
Copy link
Contributor

@oruebel oruebel left a comment

Choose a reason for hiding this comment

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

@bendichter could you add a docstring at least at the top of each module (py-file) to describe what the module contains and how to use it. It would be nice to also have docstrings for each of the functions, but I think in this particular case that is less critical as the functions are more-or-less self-describing once you know what the purpose of a mock_X function is.

Otherwise this looks good to me. Thanks for putting this together.

Copy link
Contributor

@oruebel oruebel left a comment

Choose a reason for hiding this comment

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

Can you please update the Changelog. Otherwise this looks good to me.

@bendichter bendichter merged commit 334fab9 into dev Sep 16, 2022
@oruebel oruebel deleted the dummy branch September 16, 2022 04:36
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.

[Feature]: neurodata object factories
3 participants