refactor: ♻️ read_register() takes a register name as argument, not path#299
refactor: ♻️ read_register() takes a register name as argument, not path#299lwjohnst86 wants to merge 3 commits into
read_register() takes a register name as argument, not path#299Conversation
|
|
||
| expect_error(read_register(incompatible_output), "incompatible") | ||
| expect_error(read_parquet_partition(incompatible_output), "incompatible") | ||
| }) |
There was a problem hiding this comment.
I'll add tests once the simulation helpers are merged in #295
|
|
||
| ```{r read-file} | ||
| file <- fastreg::read_register(output_file_dir) | ||
| #| eval: false |
There was a problem hiding this comment.
I'll update this once the simulation PR is done #295
signekb
left a comment
signekb
left a comment
There was a problem hiding this comment.
Very nice work 🚀 I have a hard time understanding the divide with read_register(), read_parquet_partition() and read_parquet_file(), so I've tried asking some questions about that
| #' this function doesn't work, use [read_parquet_partition()] or | ||
| #' [read_parquet_file()] instead. |
There was a problem hiding this comment.
Can't remember: Why do we need both of these?
There was a problem hiding this comment.
Hm, I kept them because you previously had them in the read_register(). But I guess we only really need the partition one.
| } | ||
| invisible(path) | ||
|
|
||
| read_parquet_partition(parquet_path) |
There was a problem hiding this comment.
Wait, how does that work? The partition function should only read in a single Parquet, right? Not a dataset?
There was a problem hiding this comment.
Hmmm, this divide doesn't make sense to me. Could this function be removed and its content moved into read_register()?
There was a problem hiding this comment.
Hmm, I not sure what you're asking. How does what work? Why would reading a partition need a single Parquet file? A partitioned Parquet dataset is a folder. Maybe, could you ask in a different way so I understand what you're meaning?
|
|
||
| ``` r | ||
| fastreg::read_register("path/to/parquet_register/") | ||
| fastreg::read_register("bef") |
github-project-automation
Bot
moved this from In review
to In progress
in Platform development
Co-authored-by: Signe Kirk Brødbæk <40836345+signekb@users.noreply.github.com>
2 participants
Description
This simplifies the interface to reading in an existing partition register, so that only the register name is required to run.
Closes #195, closes #81
Needs a thorough review.
Checklist
just run-all