Bsweger/expand moto fixtures #66
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Originally, the s3_setup fixture created in conftest.py was designed to unit test cladetime's ability to pull the correct versionId of S3 objects when provided with a specific date. Thus, the content of the objects was irrelevant. Since then, we've added Cladetime features that also require testing the content of the files on S3. This changeset updates the s3_setup pytest fixture to include realistic metadata, sequence, and ncov_metadata files. Rather than using file content to test the version, we can now check a metadata field called "version".
This seems obvious in hindsignt, but for .zst files, the sequence.get_metadata function uses polars to access URLs directly (via scan_csv). Polars uses fsspec to open remote files, so if we pass it a url to a mock, moto-created S3 bucket, it will simply try to access a real S3 bucket (hence the 403 errors) The moto setup works for .xz files, because in that case, the actual file-handling is done by requests, which then feeds the data to polars.
zkamvar
previously approved these changes
Dec 4, 2024
These additional checks do some basic asserts to ensure the schema of the metadata columns used by Cladetime and to ensure completeness/uniqueness of the strain column (which acts, essentially, as a primary key)
Thanks for the review! Some of the columns may contain missing values, since here we're checking the metadata download before it goes through cladetime's function to filter out missing/bad data. But I did add a commit that checks the data type of the columns we care about and checks the integrity of the column that acts as the metadata's unique id. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves #65
This one can be reviewed commit by commit (each commit has a descriptive message).
We could do more robust S3 mocking by running moto in server mode or figuring out to mock the fsspec interface. At this point in time, I don't think the juice is worth the sqeeze, so this PR contains a hacky workaround that will execute the code path that caused last week's bug.
Additionally, the first commit updates the mock S3 fixture to contain files with sensible data (since now we want to test file contents in addition to S3 version lookups).