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

[r] Add tests for enum value filters in SOMAExperimentAxisQuery #2316

Merged
merged 2 commits into from
Mar 25, 2024

Conversation

mojaveazure
Copy link
Member

Add a tests for doing value filters on enums in SOMAExperimentAxisQuery; test for levels both present and missing. Skip missing level tests for core < 2.21

addresses #1988

[sc-43700]

Add a tests for doing value filters on enums in
`SOMAExperimentAxisQuery`; test for levels both present and missing
Skip missing level tests for core < 2.21
Copy link

This pull request has been linked to Shortcut Story #43700: [r] Validate experiment query for value not in enumeration.

Copy link

codecov bot commented Mar 25, 2024

Codecov Report

Merging #2316 (c625c93) into main (1c1f362) will decrease coverage by 4.07%.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2316      +/-   ##
==========================================
- Coverage   78.75%   74.69%   -4.07%     
==========================================
  Files         140       52      -88     
  Lines       10756     4347    -6409     
  Branches      215        0     -215     
==========================================
- Hits         8471     3247    -5224     
+ Misses       2187     1100    -1087     
+ Partials       98        0      -98     
Flag Coverage Δ
libtiledbsoma ?
python ?
r 74.69% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
python_api ∅ <ø> (∅)
libtiledbsoma ∅ <ø> (∅)

Copy link
Member

@johnkerl johnkerl left a comment

Choose a reason for hiding this comment

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

🔥
Thanks @mojaveazure !!

@eddelbuettel
Copy link
Contributor

Should we have some for SOMADataFrame etc too?

@johnkerl
Copy link
Member

@eddelbuettel I'm happy with these as the experiment axis-query goes necessarily through dataframe

@mojaveazure
Copy link
Member Author

@eddelbuettel I don't think SOMADataFrame has a native query interface. There are already tests for SOMADataFrame that work with enums (adding, updating, and handling ordered), and the queries in this PR all go through obs, which is a SOMADataFrame

@mojaveazure mojaveazure merged commit 9d8a8fb into main Mar 25, 2024
7 checks passed
@mojaveazure mojaveazure deleted the ph/test/enum-axis-query branch March 25, 2024 18:33
github-actions bot pushed a commit that referenced this pull request Mar 25, 2024
…2316)

* [r] Add tests for enum value filters in `SOMAExperimentAxisQuery`
Add a tests for doing value filters on enums in
`SOMAExperimentAxisQuery`; test for levels both present and missing

* Add tests for handling missing levels
Skip missing level tests for core < 2.21
@eddelbuettel
Copy link
Contributor

eddelbuettel commented Mar 25, 2024

I used this to test (and posted it, here I omitted the preceding library() calls for brevity). The value_filter` does what is asked for:

uri <- tempfile()
pp <- data.frame(soma_joinid=as.integer64(1:344), penguins)
fromDataFrame(pp, uri, col_index=1)

set_log_level("debug")
#sdf <- SOMADataFrameOpen(uri)$read(value_filter="species == \"Adelie\"")$concat()
sdf <- SOMADataFrameOpen(uri)$read(value_filter="species == \"Frodo\"")$concat()
print(sdf)

cat("And done\n")

johnkerl pushed a commit that referenced this pull request Mar 25, 2024
…2316) (#2317)

* [r] Add tests for enum value filters in `SOMAExperimentAxisQuery`
Add a tests for doing value filters on enums in
`SOMAExperimentAxisQuery`; test for levels both present and missing

* Add tests for handling missing levels
Skip missing level tests for core < 2.21

Co-authored-by: Paul Hoffman <mojaveazure@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants