Skip to content

Update data source, rename variables, deprecate annotator (v0.2.0)#99

Merged
LukasWallrich merged 3 commits into
mainfrom
feature/update-data-source-and-deprecate-annotator
Feb 1, 2026
Merged

Update data source, rename variables, deprecate annotator (v0.2.0)#99
LukasWallrich merged 3 commits into
mainfrom
feature/update-data-source-and-deprecate-annotator

Conversation

@LukasWallrich
Copy link
Copy Markdown
Contributor

@LukasWallrich LukasWallrich commented Feb 1, 2026

Summary

  • Migrate to new data source URL with updated variable naming conventions
  • Update all variable names to use _o/_r suffixes (e.g., es_originales_o, n_replicationn_r)
  • Deprecate local annotator app - run_annotator() now opens web version at forrt.org

Breaking Changes

Users with existing code referencing old variable names will need to update to the new _o/_r naming convention.

Test plan

  • R CMD check passes with no errors or warnings
  • Verify new OSF data source URL is accessible
  • Test run_explorer() app functionality
  • Test run_annotator() redirects to web version

🤖 Generated with Claude Code

Breaking changes:
- New data source URL (osf.io/2tbvd) with updated variable naming
- Variable names now use _o/_r suffixes (e.g., es_original → es_o)
- run_annotator() deprecated - now opens web version at forrt.org

Other changes:
- Add MIT LICENSE file
- Update explorer app for new column names
- Remove local annotator app (inst/fred_annotator/)
- Update vignettes and documentation

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Feb 1, 2026

Claude finished @LukasWallrich's task —— View job


PR Review Progress

Reviewing this substantial PR with 241 additions and 2052 deletions across 46 files.

Todo List:

  • Read and understand repository structure and conventions
  • Analyze variable renaming changes for consistency
  • Review data source migration implementation
  • Evaluate annotator deprecation approach
  • Check code quality and best practices
  • Assess security considerations
  • Review test coverage and documentation updates
  • Provide comprehensive feedback

Code Review Summary

This is a significant and well-executed refactoring that implements several important breaking changes for v0.2.0. The changes are logical, well-documented, and maintain backwards compatibility where appropriate.

✅ Strengths

1. Variable Renaming Implementation

  • Consistent approach: All variable names correctly updated from verbose _original/_replication to concise _o/_r suffixes
  • Thorough coverage: Changes applied across all R files, tests, documentation, and Shiny apps
  • Good examples: es_originales_o, n_replicationn_r, ref_originalref_o

2. Data Source Migration

  • Clean URL update: New OSF data source (https://osf.io/2tbvd/download) properly implemented in R/zzz_parameters.R:25
  • Robust error handling: Fallback to inbuilt data if download fails (data_management.R:118-120)
  • Security: URLs are from trusted domain (OSF)

3. Annotator Deprecation Strategy

  • Proper deprecation: Uses lifecycle::badge("deprecated") and .Deprecated() function
  • User-friendly transition: Automatically opens web version with clear messaging
  • Clean removal: All 13 local annotator files properly removed from inst/fred_annotator/

4. Documentation Quality

  • Comprehensive NEWS.md: Clear breaking changes documentation
  • Updated function docs: All parameter names updated (e.g., man/align_effect_direction.Rd:4)
  • Version bump: Properly incremented to 0.2.0

🔍 Code Quality Observations

Security

  • Download functions properly use environment variables
  • No hardcoded credentials or sensitive data
  • Source operations are limited to trusted local files

Error Handling

  • Robust try-catch blocks in data loading functions
  • Graceful fallbacks to offline data when network fails
  • Clear error messages for users

Testing

  • Existing test suite covers core functionality
  • Tests properly updated for new variable names
  • Test coverage appears adequate for the changes

⚠️ Minor Considerations

1. Test Plan Completion

  • PR shows incomplete test plan items:
    • Verify new OSF data source URL is accessible
    • Test run_explorer() app functionality
    • Test run_annotator() redirects to web version

2. Potential User Impact

  • Breaking changes are well-documented but significant
  • Consider if a migration guide would help users update their code

3. Data Validation

  • The new data source structure should be validated to ensure all expected columns are present
  • numeric_variables definition in data_management.R:109 could use validation against actual data schema

📋 Recommendations

  1. Complete test plan: Verify the remaining test plan items before merging
  2. Add migration helper: Consider a utility function to help users migrate old variable names
  3. Data validation: Add schema validation for the new data source format

🎯 Overall Assessment

This is a high-quality PR that successfully implements major architectural improvements:

  • ✅ Well-planned breaking changes with clear documentation
  • ✅ Consistent implementation across the entire codebase
  • ✅ Proper deprecation strategy for the annotator
  • ✅ Robust error handling and fallback mechanisms
  • ✅ No security concerns identified

The 2052 deletions (removing the local annotator) significantly reduce maintenance burden while the new variable naming convention improves API consistency. Recommended for approval after completing the remaining test plan items.


LukasWallrich and others added 2 commits February 1, 2026 17:15
Skip documentation commit/push on PRs (detached HEAD causes git errors).
Skip entire document job on PRs in R-CMD-check since docs are rebuilt
during the check anyway. Use explicit branch refs for push operations.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Include pval_value_o and pval_value_r in numeric_variables
to ensure consistent type handling with other numeric fields.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Feb 1, 2026

Claude finished @LukasWallrich's task —— View job


Claude finished @LukasWallrich's task —— View job


PR Review Progress

Reviewing this substantial PR with 241 additions and 2052 deletions across 46 files.

Todo List:

  • Read and understand repository structure and conventions
  • Analyze variable renaming changes for consistency
  • Review data source migration implementation
  • Evaluate annotator deprecation approach
  • Check code quality and best practices
  • Assess security considerations
  • Review test coverage and documentation updates
  • Provide comprehensive feedback

Code Review Summary

This is a significant and well-executed refactoring that implements several important breaking changes for v0.2.0. The changes are logical, well-documented, and maintain backwards compatibility where appropriate.

✅ Strengths

1. Variable Renaming Implementation

  • Consistent approach: All variable names correctly updated from verbose _original/_replication to concise _o/_r suffixes
  • Thorough coverage: Changes applied across all R files, tests, documentation, and Shiny apps
  • Good examples: es_originales_o, n_replicationn_r, ref_originalref_o

2. Data Source Migration

  • Clean URL update: New OSF data source (https://osf.io/2tbvd/download) properly implemented in R/zzz_parameters.R:25
  • Robust error handling: Fallback to inbuilt data if download fails (data_management.R:118-120)
  • Security: URLs are from trusted domain (OSF)

3. Annotator Deprecation Strategy

  • Proper deprecation: Uses lifecycle::badge("deprecated") and .Deprecated() function
  • User-friendly transition: Automatically opens web version with clear messaging
  • Clean removal: All 13 local annotator files properly removed from inst/fred_annotator/

4. Documentation Quality

  • Comprehensive NEWS.md: Clear breaking changes documentation
  • Updated function docs: All parameter names updated (e.g., man/align_effect_direction.Rd:4)
  • Version bump: Properly incremented to 0.2.0

🔍 Code Quality Observations

Security

  • Download functions properly use environment variables
  • No hardcoded credentials or sensitive data
  • Source operations are limited to trusted local files

Error Handling

  • Robust try-catch blocks in data loading functions
  • Graceful fallbacks to offline data when network fails
  • Clear error messages for users

Testing

  • Existing test suite covers core functionality
  • Tests properly updated for new variable names
  • Test coverage appears adequate for the changes

⚠️ Minor Considerations

1. Test Plan Completion

  • PR shows incomplete test plan items:
    • Verify new OSF data source URL is accessible
    • Test run_explorer() app functionality
    • Test run_annotator() redirects to web version

2. Potential User Impact

  • Breaking changes are well-documented but significant
  • Consider if a migration guide would help users update their code

3. Data Validation

  • The new data source structure should be validated to ensure all expected columns are present
  • numeric_variables definition in data_management.R:109 could use validation against actual data schema

📋 Recommendations

  1. Complete test plan: Verify the remaining test plan items before merging
  2. Add migration helper: Consider a utility function to help users migrate old variable names
  3. Data validation: Add schema validation for the new data source format

🎯 Overall Assessment

This is a high-quality PR that successfully implements major architectural improvements:

  • ✅ Well-planned breaking changes with clear documentation
  • ✅ Consistent implementation across the entire codebase
  • ✅ Proper deprecation strategy for the annotator
  • ✅ Robust error handling and fallback mechanisms
  • ✅ No security concerns identified

The 2052 deletions (removing the local annotator) significantly reduce maintenance burden while the new variable naming convention improves API consistency. Recommended for approval after completing the remaining test plan items.


@LukasWallrich LukasWallrich merged commit 69b855f into main Feb 1, 2026
5 of 8 checks passed
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.

1 participant