Conversation
Merge branch 'dev' of https://github.com/ices-tools-dev/RDBEScore into dev # Conflicts: # R/lowerTblData.R # R/upperTblData.R # man/lowerTblData.Rd # man/upperTblData.Rd
- fixed issue with LE table in hierarchies 7 and 9 - added gc() in some points but code optimalization needed. - #issue 155
This reverts commit cea8ae3.
Fix test coverage works
Fix test version
…code rewritten to use data.table rather than dplyr (#233)
…sign variables get included in the estimation object output (#233)
renamed vignettes
"see other packages" section added with all other vignettes #238
vignettes should not start with number #244
There was a problem hiding this comment.
Pull Request Overview
This pull request standardizes vignette titles, improves cross-referencing between vignettes, fixes spelling errors, and removes obsolete test data directories. The changes focus primarily on documentation consistency and test data cleanup.
Key Changes:
- Standardized vignette titles with numeric prefixes (e.g., "01a", "02b") for better organization
- Added dynamic cross-referencing sections to all vignettes listing related documentation
- Fixed spelling errors ("prevenes" → "prevents")
- Removed two obsolete H7 test data directories (v1.19.18 and v1.19.26)
- Enhanced test coverage for validation rules, import functionality, and estimation object creation
Reviewed Changes
Copilot reviewed 88 out of 125 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| vignettes/*.Rmd (8 files) | Updated titles with numeric prefixes and added dynamic vignette cross-reference sections |
| tests/testthat/test-validateRDBESDataObject.R | Added tests for SL/IS validation rules |
| tests/testthat/test-importRDBESDataZIP.R | New test file for ZIP import functionality |
| tests/testthat/test-importRDBESDataCSV.R | New test file for CSV import functionality |
| tests/testthat/test-createRDBESEstObject.R | Added SSid assignments for parent-child relationships |
| tests/testthat/test-createRDBESDataObject.R | Updated tests to expect clear error messages for malformed data |
| tests/testthat/h7_v_1_19_26/* (11 files) | Removed obsolete H7 v1.19.26 test data directory |
| tests/testthat/h7_v_1_19_18/* (10 files) | Removed obsolete H7 v1.19.18 test data directory |
| man/upperTblData.Rd | Removed unnecessary blank line in examples section |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| The difference between these two cases can be specified via the argument `overwriteSampled` in the function `generateNAsUsingSL`. By default (estimation case) the argument is set to TRUE which makes `generateNAsUsingSL` set the weights of these extra species to NA. But, by explicitly setting that argument as `overwriteSampled=FALSE` the information collected can also kept. | ||
|
|
||
| To demonstrate this we carry out a small alteration of the example data, removing the *Nephrops norvegicus* from the Species List. This creates a somewhat atypical situation (it configures a case where of a haul where nothing was supposed to be looked for but still *Nephrops norvegicus* was registered) that is used here for sake of simplifying the example. | ||
| To demonstrate this we carry out a small alteration of the example data, removing the *Nephrops norvegicus* from the Species List. This creates a somewhat atypical situation: a haul where nothing was supposed to be looked for but still *Nephrops norvegicus* was registered. We use such situation here for sake of a simple example. |
There was a problem hiding this comment.
The phrasing 'for sake of a simple example' should be 'for the sake of a simple example' to be grammatically correct.
Suggested change
| To demonstrate this we carry out a small alteration of the example data, removing the *Nephrops norvegicus* from the Species List. This creates a somewhat atypical situation: a haul where nothing was supposed to be looked for but still *Nephrops norvegicus* was registered. We use such situation here for sake of a simple example. | |
| To demonstrate this we carry out a small alteration of the example data, removing the *Nephrops norvegicus* from the Species List. This creates a somewhat atypical situation: a haul where nothing was supposed to be looked for but still *Nephrops norvegicus* was registered. We use such situation here for the sake of a simple example. |
erosquesada
approved these changes
Oct 15, 2025
rix133
added a commit
that referenced
this pull request
Oct 15, 2025
Merge pull request #244 from ices-tools-dev/dev
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
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 pull request introduces several improvements and optimizations to the RDBEScore package, focusing on estimation object creation, memory management, and documentation. The main changes include more efficient handling of sub-sampling relationships, an option to exclude design variables for smaller objects, improved joining logic, and the addition of a GitHub Actions workflow for pkgdown site deployment.
Estimation object creation and memory management:
incDesignVariablesparameter tocreateRDBESEstObject()to allow users to exclude design variables, reducing object size. Character columns are now converted to factors for further memory optimization. [1] [2] [3]SAtable by replacing recursive logic with a more efficient self-join and lookup table (prepareSubSampleLevelLookup), including warnings for missing or non-unique matches. [1] [2] [3]gc()(garbage collection) throughout the estimation object creation and joining functions to reduce memory usage during large data processing. [1] [2] [3] [4] [5] [6]Joining logic and table handling:
procRDBESEstObjUppHier()and added comments about data.table vs. dplyr joins for future optimization. [1] [2]VDidfields to avoid confusion when multiple fields are present.Documentation and workflow:
.github/workflows/pkgdown.yaml) for automated pkgdown site building and deployment to GitHub Pages..Rbuildignoreto exclude pkgdown config and output files.Miscellaneous:
DESCRIPTIONfile to include the pkgdown site.filterRDBESDataObject()to use data.table for efficiency.