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

Specify an input glob pattern #16

Merged
merged 6 commits into from
Apr 12, 2022

Conversation

iaindillingham
Copy link
Member

@iaindillingham iaindillingham commented Apr 8, 2022

This PR allows us to specify an input glob pattern, rather than an input path, allowing deciles-charts to read a subset of the measure tables.

94e233e addresses the substantive issue; the other commits either fix a broken test 😳 or tidy up.

Closes #7

The `test_parse_args` test is broken because it doesn't contain any
assert statements.
This replaces the `--input-dir` argument with the `--input-files`
argument. Whereas the former accepts a path to a directory, the latter
accepts a glob pattern.

This commit addresses the substantive issue, but some tidying up would
be worthwhile.

Closes #7
Here, this means all paths from args are now resolved (i.e. made
absolute).
We can remove `test_input_table`, because `test_measure_table` now tests
that other CSVs (either weekly/monthly CSVs or CSVs containing input
tables) are filtered.
As @inglesp pointed out, the call to `next` is distracting. If we want
to advance the generator function, then we can instead call `list`. As
a bonus, calling `list` exhausts the generator function, so we can
assert that the expected number of measure tables have been returned.
Copy link

@ccunningham101 ccunningham101 left a comment

Choose a reason for hiding this comment

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

Tested on my antidepressant project as well.
Looks good!

@iaindillingham iaindillingham merged commit b9cd33b into main Apr 12, 2022
@iaindillingham iaindillingham deleted the iaindillingham/input-glob-pattern branch April 12, 2022 08:38
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.

Specify an input glob pattern
2 participants