Skip to content

Refactor repeated checks in colspace internal fcts - #263

Merged
Bisaloo merged 7 commits into
masterfrom
check-function-colspace
Aug 1, 2025
Merged

Bisaloo merged 7 commits into
masterfrom
check-function-colspace

Conversation

@Bisaloo

@Bisaloo Bisaloo commented Jul 31, 2025

Copy link
Copy Markdown
Collaborator

Fixes #102, fixes #192

Prepares for #208 & #179

@Bisaloo
Bisaloo force-pushed the check-function-colspace branch from 5d99549 to cbc71e0 Compare July 31, 2025 20:04
@Bisaloo
Bisaloo marked this pull request as ready for review July 31, 2025 20:05
@Bisaloo
Bisaloo force-pushed the check-function-colspace branch from cbc71e0 to 9592c0d Compare August 1, 2025 10:16
@Bisaloo
Bisaloo requested a review from Copilot August 1, 2025 10:21

This comment was marked as outdated.

@Bisaloo
Bisaloo requested review from Copilot and thomased August 1, 2025 10:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request refactors colorspace internal functions to eliminate duplicated validation logic and standardizes error messages. The main change introduces a new helper function check_data_for_colspace that consolidates repeated data validation checks across multiple colorspace functions.

Key changes:

  • Introduces check_data_for_colspace helper function to centralize data validation
  • Updates error/warning messages for consistency (e.g., "three" → "3", "tetrachromatic" → "tetrachromat")
  • Refactors 7 colorspace functions to use the new helper instead of duplicated validation code

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
R/check_data_for_colspace.R New helper function that consolidates data validation logic for colorspace functions
R/trispace.R Refactored to use new helper function, removing 50+ lines of duplicated validation code
R/tcspace.R Refactored to use new helper function, removing 50+ lines of duplicated validation code
R/segspace.R Refactored to use new helper function, removing 40+ lines of duplicated validation code
R/hexagon.R Refactored to use new helper function, removing 40+ lines of duplicated validation code
R/dispace.R Refactored to use new helper function, removing 60+ lines of duplicated validation code
R/colspace.R Minor improvements to error handling and relative quantum catch detection
R/coc.R Refactored to use new helper function, removing 50+ lines of duplicated validation code
R/categorical.R Refactored to use new helper function, removing 40+ lines of duplicated validation code
R/sensmodel.R Added missing call. = FALSE to error message
R/getimg.R Improved file extension checking logic
tests/testthat/test-colspace.R Updated test expectations to match new standardized error messages
.lintr Updated linter configuration

Comment thread R/check_data_for_colspace.R Outdated
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@thomased

thomased commented Aug 1, 2025

Copy link
Copy Markdown
Collaborator

Woah! This is awesome - thanks. I’ve had a poke around locally and not found anything of note - happy to merge when you’re satisfied.

Also the copilot reviews are very cool, that’s new to me.

@Bisaloo
Bisaloo merged commit 7a55065 into master Aug 1, 2025
@Bisaloo
Bisaloo deleted the check-function-colspace branch August 1, 2025 12:52
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.

Make colspace() work when columns are not in expected order Validate/sanitize inputs more generally

3 participants