Merged
Conversation
this is derived from the original by antoine adde
plus adding a with_tables function
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements transition-predictor relationship functionality and performs a significant refactor of the evoland_db class architecture. The main changes extract generic database operations into a new parquet_duckdb base class and add covariance filtering for predictor selection.
Key changes:
- Extract generic parquet/DuckDB operations into new
parquet_duckdbbase class - Implement
create_trans_preds_t()with covariance filtering for predictor selection - Add
covariance_filter()function for two-stage variable selection - Add neighbor analysis capabilities with
neighbors_ttable - Refactor
cast_dt_col()to use type strings instead of functions
Reviewed changes
Copilot reviewed 30 out of 30 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| R/parquet_duckdb.R | New base class extracting generic database operations from evoland_db |
| R/evoland_db.R | Refactored to inherit from parquet_duckdb, removing duplicated code |
| R/evoland_db_tables.R | New file organizing table active bindings using R6's $set() |
| R/evoland_db_views.R | Refactored to use R6's $set() for view bindings and add trans_pred_data_v method |
| R/evoland_db_neighbors.R | New neighbor analysis methods for evoland_db |
| R/trans_preds_t.R | Implements create_trans_preds_t with covariance filtering logic |
| R/trans_meta_t.R | Removes id_trans attribution (now handled by DB layer) |
| R/covariance_filter.R | New two-stage covariance filtering implementation |
| R/util.R | Refactors cast_dt_col to use type strings with switch statement |
| R/neighbors_t.R | New table class for neighbor relationships |
| inst/tinytest/test_trans_preds_t.R | Comprehensive integration tests for trans_preds_t |
| inst/tinytest/test_parquet_duckdb.R | Complete test suite for parquet_duckdb base class |
| inst/tinytest/test_evoland_db.R | Updated tests focusing on domain-specific functionality |
| DESCRIPTION | Adds explicit Collate field for source file ordering |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 38 out of 38 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
This will close #9 once done.
This ended up including a big refactor for the
evoland_dbgenerator object, making explicit file collation (sourcing order) necessary.