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

Pytests round 2 #851

Merged
merged 6 commits into from
Mar 7, 2024
Merged

Pytests round 2 #851

merged 6 commits into from
Mar 7, 2024

Conversation

CBroz1
Copy link
Member

@CBroz1 CBroz1 commented Feb 26, 2024

Description

Adds coverage to pytests across common, lfp, utils

Coverage report
Name                                         Cover 
--------------------------------------------------
common/common_behav.py                        82% 
common/common_device.py                       91% 
common/common_dio.py                          94% 
common/common_ephys.py                        47% 
common/common_filter.py                       90% 
common/common_interval.py                     98% 
common/common_lab.py                          93% 
common/common_nwbfile.py                      60% 
common/common_position.py                     65% 
common/common_region.py                      100%
common/common_ripple.py                        0% 
common/common_sensors.py                      92% 
common/common_session.py                      88% 
common/common_subject.py                      80% 
common/common_task.py                         80% 
common/common_usage.py                       100%
common/errors.py                             100%
common/populate_all_common.py                100%
common/prepopulate/prepopulate.py             30% 
common/signal_processing.py                    0% 
lfp/analysis/v1/lfp_band.py                   91% 
lfp/lfp_electrode.py                         100%
lfp/lfp_imported.py                          100%
lfp/lfp_merge.py                             100%
lfp/v1/lfp.py                                 94% 
lfp/v1/lfp_artifact.py                        58% 
lfp/v1/lfp_artifact_MAD_detection.py          22% 
lfp/v1/lfp_artifact_difference_detection.py   13% 
linearization/merge.py                       100%
linearization/v0/main.py                      51% 
linearization/v1/main.py                      83% 
position/position_merge.py                    39% 
position/v1/dlc_decorators.py                 50% 
position/v1/dlc_reader.py                     23% 
position/v1/dlc_utils.py                      13% 
position/v1/position_dlc_centroid.py          17% 
position/v1/position_dlc_cohort.py            43% 
position/v1/position_dlc_model.py             35% 
position/v1/position_dlc_orient.py            29% 
position/v1/position_dlc_pose_estimation.py   25% 
position/v1/position_dlc_position.py          21% 
position/v1/position_dlc_project.py           20% 
position/v1/position_dlc_selection.py         30% 
position/v1/position_dlc_training.py          28% 
position/v1/position_trodes_position.py       59% 
utils/database_settings.py                    85% 
utils/dj_chains.py                            89% 
utils/dj_helper_fn.py                         78% 
utils/dj_merge_tables.py                      74% 
utils/dj_mixin.py                             78% 
utils/logging.py                              88% 
utils/nwb_helper_fn.py                        82% 
--------------------------------------------------
TOTAL                                         55%
# Checklist:
  • This PR should be accompanied by a release: no
  • (If release) I have updated the CITATION.cff
  • I have updated the CHANGELOG.md
  • I have added/edited docs/notebooks to reflect the changes

src/spyglass/lfp/analysis/v1/lfp_band.py Outdated Show resolved Hide resolved
src/spyglass/lfp/analysis/v1/lfp_band.py Outdated Show resolved Hide resolved
src/spyglass/utils/nwb_helper_fn.py Outdated Show resolved Hide resolved
tests/conftest.py Show resolved Hide resolved
from spyglass.linearization.merge import LinearizedPositionOutput
from spyglass.position.position_merge import PositionOutput

# must import common_position before LinOutput to avoid circular import
Copy link
Member Author

Choose a reason for hiding this comment

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

A byproduct of the V0 move is that LinearizedOutput can't be imported before the deprecated items in common_position. Not sure if it counts as a bug worth opening an issue for

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is worth opening up an issue for. It's not really expected behavior and will cause confusion.

tests/lfp/test_pipeline.py Show resolved Hide resolved
tests/linearization/conftest.py Show resolved Hide resolved
tests/utils/test_nwb_helper_fn.py Show resolved Hide resolved
@CBroz1 CBroz1 marked this pull request as ready for review February 29, 2024 00:32
src/spyglass/lfp/analysis/v1/lfp_band.py Outdated Show resolved Hide resolved
src/spyglass/lfp/analysis/v1/lfp_band.py Outdated Show resolved Hide resolved
src/spyglass/lfp/analysis/v1/lfp_band.py Show resolved Hide resolved
src/spyglass/utils/nwb_helper_fn.py Outdated Show resolved Hide resolved
from spyglass.linearization.merge import LinearizedPositionOutput
from spyglass.position.position_merge import PositionOutput

# must import common_position before LinOutput to avoid circular import
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is worth opening up an issue for. It's not really expected behavior and will cause confusion.

@edeno
Copy link
Collaborator

edeno commented Mar 7, 2024

Seems good to go pending resolution of conflicts.

@CBroz1 CBroz1 requested a review from edeno March 7, 2024 20:39
@edeno edeno merged commit d34bb04 into LorenFrankLab:master Mar 7, 2024
7 checks passed
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.

2 participants