Skip to content

Fix lint issues in report.factor.R and add .lintr configuration file#561

Merged
rempsyc merged 6 commits intomainfrom
copilot/fix-560
Sep 11, 2025
Merged

Fix lint issues in report.factor.R and add .lintr configuration file#561
rempsyc merged 6 commits intomainfrom
copilot/fix-560

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Sep 10, 2025

This PR resolves the linting violations in R/report.factor.R using the correct easystats lintr configuration from the centralized workflows and adds a .lintr file for easier copilot access.

Issues Fixed

Object Overwrite Violations: Renamed variables to avoid conflicts with base R functions:

New Addition: .lintr Configuration File

Added .lintr file with the exact easystats configuration extracted from easystats/workflows repository to:

  • ✅ Provide single source of truth for linting configuration
  • ✅ Make it easier for future copilot agents to access correct settings
  • ✅ Eliminate need to parse configuration from copilot instructions
  • ✅ Ensure perfect CI/local linting consistency

Key Insight

The original issue was that I used an incorrect linter configuration that didn't match the actual easystats standards:

Previous (incorrect): Used default linting with 80-character line limit
Current (correct): Used proper easystats configuration with 120-character line limit and proper linter settings

The easystats configuration correctly allows 120 characters per line, so the "line length violations" in my original fix were unnecessary. The real issues were only the object overwrite warnings.

Verification

All factor reporting functionality continues to work correctly:

library(report)

# Basic factor reporting
test_factor <- factor(c("Low", "Medium", "High", "Low", "High", "Medium", "Low"))
report(test_factor)
#> x: 3 levels, namely High (n = 2, 28.57%), Low (n = 3, 42.86%) and Medium (n = 2, 28.57%)

# Logical data (uses factor methods)  
test_logical <- c(TRUE, FALSE, TRUE, TRUE, FALSE, FALSE)
report(test_logical)
#> x: 2 levels, namely FALSE (n = 3, 50.00%) and TRUE (n = 3, 50.00%)

# All component functions work correctly
report_parameters(test_factor)
report_statistics(test_factor)
report_table(test_factor)
report_text(test_factor)

Linter Result: 0 issues found with correct easystats configuration
Package Build: Successful
Tests: All factor-related tests pass
Functionality: All report.factor, report.logical, and report.Date methods work correctly
.lintr file: Tested and verified working with local linting

The changes are minimal and surgical, addressing only the real linting issues according to the proper easystats standards.

Future Usage

For future requests, specify: "Use the .lintr file in the repository root as the authoritative linting configuration" to ensure copilot agents use the local configuration.

Fixes #560.


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

Co-authored-by: rempsyc <13123390+rempsyc@users.noreply.github.com>
Copilot AI changed the title [WIP] [Copilot]: Fix lint issues in report.factor.R Fix lint issues in report.factor.R Sep 10, 2025
Copilot AI requested a review from rempsyc September 10, 2025 04:08
@rempsyc
Copy link
Copy Markdown
Member

rempsyc commented Sep 10, 2025

@copilot the original issue mentioned "Use linters configuration from instructions". But it seems you haven't used the repo special linters configuration from instructions. Are your instructions too long now and you miss them? Would it help if instead we added a linters file in the repo with the correct configuration? It wouldn't be synced with the easystats organization workflow configuration, sadly, but at least, might it help you stop missing them? Think of the best long-term solution for this problem for the future copilot agents, etc.

Co-authored-by: rempsyc <13123390+rempsyc@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Sep 10, 2025

@copilot the original issue mentioned "Use linters configuration from instructions". But it seems you haven't used the repo special linters configuration from instructions. Are your instructions too l...

You're absolutely right! I discovered the issue: the object_overwrite_linter is missing from the copilot instructions' linter configuration, causing a mismatch with the actual easystats CI workflow.

The problem: My original fix was based on incomplete linter configuration (missing object_overwrite_linter) and incorrectly assumed 80-character line limits instead of the proper 120-character easystats standard.

The fix: I've now applied the correct easystats configuration and addressed only the real issues - object overwrite warnings by renaming tablereport_table_obj and textreport_text_obj (following the pattern from PR #558).

Long-term solution: A local .lintr file would indeed help, but requires careful syntax. However, the root issue is that the copilot instructions need to be updated to include the missing object_overwrite_linter setting to match the actual easystats workflow.

The cleanest solution would be to sync the copilot instructions with the exact easystats/workflows configuration to prevent future mismatches. Commit 94e935b now uses the correct configuration and passes all linting checks.

Copilot AI changed the title Fix lint issues in report.factor.R Fix lint issues in report.factor.R using correct easystats configuration Sep 10, 2025
@rempsyc
Copy link
Copy Markdown
Member

rempsyc commented Sep 10, 2025

@copilot ok thanks, now create a proper linter file (with our desired configuration) that will be EASIER for copilot to access than finding it from the instructions (since it seems copilot doesn't always read it completely). Also give me an example of how to specify in my future requests that I want the copilot agent to use that file as authoritative reference. If at all helpful, consider this old file of us that we had and which I deleted because I thought you would get it from instructions (perhaps adapt instructions to say to look at that file instead of mentioning in instructions to reduce redundancy and possible conflicts):

https://github.com/easystats/report/blob/ce3b8dcdf6b6991933d827e79e6b633632274581/.lintr

Co-authored-by: rempsyc <13123390+rempsyc@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Sep 10, 2025

@copilot ok thanks, now create a proper linter file (with our desired configuration) that will be EASIER for copilot to access than finding it from the instructions (since it seems copilot doesn't alw...

I've created a comprehensive .lintr file (commit bd6a610) with the exact easystats configuration from your centralized workflows repository.

For future requests, specify: "Use the .lintr file in the repository root as the authoritative linting configuration" - this will ensure copilot agents use the local configuration instead of parsing it from instructions.

Key benefits:

  • ✅ Matches CI exactly (extracted from easystats/workflows/.github/workflows/lint-changed-files.yaml)
  • ✅ 120-character line limit (not 80)
  • ✅ Complete linter configuration including all NULL settings
  • ✅ Tested and verified working locally

Configuration highlights:

  • Line length: 120 characters (easystats standard)
  • Cyclomatic complexity: 40 (vs default 15)
  • Object length: 50 characters max
  • Key disabled linters: object_name_linter, object_usage_linter, indentation_linter

This eliminates the copilot instruction parsing issue and provides a single source of truth that's easier to maintain and update.

Copilot AI changed the title Fix lint issues in report.factor.R using correct easystats configuration Fix lint issues in report.factor.R and add .lintr configuration file Sep 10, 2025
@rempsyc rempsyc marked this pull request as ready for review September 11, 2025 00:42
@rempsyc rempsyc merged commit eeca6ba into main Sep 11, 2025
24 of 26 checks passed
@rempsyc rempsyc deleted the copilot/fix-560 branch September 11, 2025 01:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Copilot]: Fix lint issues in report.factor.R

2 participants