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

Context tests #647

Merged
merged 1 commit into from
Aug 9, 2022
Merged

Context tests #647

merged 1 commit into from
Aug 9, 2022

Conversation

ackagel
Copy link
Contributor

@ackagel ackagel commented Aug 9, 2022

Purpose

Adds basic coverage tests for context dependent spatial enrichment analysis

Implementation

Uses existing enrichment testing infrastructure to test for the errorless operation of enrichment functions when provided context arguments.

Remaining issues

Functionality tests should be added in a future PR. Using tools like pytest-cases + fixtures to clean up cell table generation intest_utils.py would really clean up (at a minimum) spatial_analysis_tests.py, and would make adding functionality tests (context dependence, feature distance-hack, etc) much easier.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@ackagel ackagel changed the base branch from master to spatial-analysis-nb-update August 9, 2022 22:04
@ackagel
Copy link
Contributor Author

ackagel commented Aug 9, 2022

@ngreenwald I decided to just quarantine the context cell-table for its own test, rather than add to the existing complexity of the enriched cell-table generation code.

When functionality tests are added (post-workshop PR?), I think the enriched cell-table generation in test_utils.py should be reworked w/ tools like pytest-cases, i.e made more similar to toffy's tests. It'll be easier to add in context enrichment generation or any future cases, and would also help in the case of potential cell-table format changes (i.e no locally hardcoded column locations+names).

@ackagel ackagel requested a review from ngreenwald August 9, 2022 22:46
@ngreenwald
Copy link
Member

Sounds good, agreed that whole area of the codebase could use some attention with out newfound knowledge

@ngreenwald ngreenwald merged commit 55758bd into spatial-analysis-nb-update Aug 9, 2022
@ngreenwald ngreenwald deleted the context-tests branch August 9, 2022 23:06
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