OpenSpec: Rename Create_spc_chart() To Bfh_qic() Discussion

by Admin 60 views
OpenSpec: Streamlining Function Names - `create_spc_chart()` to `bfh_qic()`

Hey everyone! Let's dive into a proposal to rename a key function within our OpenSpec project. This might seem like a small change, but it's all about making our code more intuitive, easier to use, and better aligned with the broader ecosystem. So, let's break down why we're considering this, what it entails, and how it impacts you.

Why the Change? Current Naming Conventions

Currently, we have a function called create_spc_chart(). While it does the job, it's not the most elegant or user-friendly name out there. Here’s why we think a change is in order:

  • Too verbose: At 17 characters, it's a bit of a mouthful for a primary API function. We want something snappier!
  • Verb-heavy: The word "create" is a bit redundant. Most tidyverse/ggplot2 functions are concise and get straight to the point.
  • Not immediately recognizable: For those familiar with qicharts2, the current name doesn't immediately ring a bell. We want to bridge that gap.

The package is fundamentally a BFH-themed wrapper around qicharts2::qic() that:

  • Adds hospital branding via BFHtheme integration
  • Provides intelligent label placement
  • Simplifies the API with smart defaults
  • Returns composable ggplot2 objects

Better alignment with design philosophy: This package is essentially a BFH-flavored wrapper around qicharts2::qic(). It jazzes things up with hospital branding through BFHtheme integration, intelligently places labels, simplifies the API with smart defaults, and returns composable ggplot2 objects. Essentially, we're making it easier and more visually appealing to create statistical process control charts within the BFH context.

The Proposed Solution: bfh_qic()

So, what's the alternative? We're proposing to rename the function to bfh_qic(). Here's why we think this is a winner:

  • Branded prefix (bfh_): This clearly signals that this is BFH's version of the function.
  • Recognizable: Users familiar with qicharts2 will immediately see the connection.
  • Concise: At just 7 characters, it's much easier to type and read.
  • Consistent: It aligns with the package's identity (BFHcharts).
  • Efficient: It's much more efficient for interactive use and writing code.

We did consider spc_chart() as an alternative, which is more self-explanatory. However, it loses the qicharts2 connection and, more importantly, the BFH branding. And guys, branding matters!

What Changes are Involved?

Okay, so we're changing the name. What does that actually mean in terms of code changes? Here’s the breakdown:

Code Changes

  1. Rename the function in R/create_spc_chart.R:
    • create_spc_chart() will become bfh_qic()
    • We'll update the roxygen2 documentation, including @name, @title, and @description.
    • We'll maintain all existing parameters and behavior. No functionality changes here!
  2. Update NAMESPACE (via devtools::document()):
    • The export will change from export(create_spc_chart) to export(bfh_qic). This is how R knows the function is available to users.
  3. Update all internal references:
    • We'll update @seealso references in other functions to point to the new name.
    • We'll update the package documentation (BFHcharts-package.R) to reflect the change.
  4. Update examples and tests:
    • All test files referencing create_spc_chart() will be updated.
    • All example code in roxygen2 comments will be updated.
    • Demo scripts (demo[*.R) will be updated.
  5. Update documentation files:
    • We'll update CLAUDE.md project instructions.
    • We'll update openspec/project.md conventions.
    • We'll update README.md (if it exists). Basically, anywhere the old name appears, we'll replace it with the new one.

Breaking Changes

Yes, this is a breaking change. Any existing code that uses create_spc_chart() will fail until it's updated. However, because we're still in the early stages of development (version 0.x.x), breaking changes are acceptable. We'll bump the minor version from 0.1.x to 0.2.0 to reflect this.

  • Deprecation strategy: Not applicable. We're not going to go through a deprecation period. Since the package is pre-1.0 and the user (maintainer) will handle the SPCify migration directly, a clean break is preferred over the overhead of deprecation.

Impact Assessment

So, who and what will this change affect? Let's break it down:

  • Affected specs: public-api - This is the main exported function signature, so it's directly affected.
  • Affected code:
    • R/create_spc_chart.R - This is where the main function definition lives, so it's the primary target of the rename.
    • R/BFHcharts-package.R - The package documentation needs to be updated.
    • R/*.R - All @seealso cross-references need to be updated.
    • tests/testthat/test-*.R - All test files need to be updated to use the new name.
    • demo_*.R - Demo scripts need to be updated.
    • NAMESPACE - This file is auto-generated via roxygen2 and will be updated automatically.
  • Affected external projects:
    • SPCify: This is a Shiny application that uses this package. The user (maintainer) will handle the migration. No coordination is needed beyond this proposal.

Migration Path

The migration path is super simple: it's a find-and-replace operation. Just replace create_spc_chart with bfh_qic. The function signature is unchanged, so it's a drop-in replacement. All parameters, defaults, and behavior remain identical. Easy peasy!

Validation and Testing

Before we roll this out, we need to make sure everything is working as expected. Here's our testing strategy:

  • Run the full test suite: We'll use devtools::test() to ensure all tests pass.
  • Run a package check: We'll use devtools::check() to look for any potential issues.
  • Verify all examples run: We'll check the examples in man/*.Rd to make sure they work.
  • Test in SPCify locally: The user (maintainer) will be responsible for testing the changes in SPCify.

We'll also verify the documentation to ensure:

  • [ ] All roxygen2 docs are updated.
  • [ ] The NAMESPACE is regenerated correctly.
  • [ ] There are no broken @seealso cross-references.
  • [ ] Examples in ?bfh_qic work.

Timeline

We're planning to implement this change in a single PR, with all changes bundled together. Deployment will be immediate, with no deprecation period. As mentioned earlier, we'll bump the minor version from 0.1.x to 0.2.0.

  • Implementation: Single PR, all changes together
  • Deployment: Immediate (no deprecation period)
  • Version bump: Minor (0.1.x → 0.2.0)

Related Issues

We'll create a GitHub issue to track this proposal and any related discussions. Stay tuned for the link!

Summary

So, that's the proposal! We believe renaming create_spc_chart() to bfh_qic() will make our code more user-friendly, consistent, and aligned with the broader ecosystem. It's a small change with a big impact on usability and maintainability. Let us know your thoughts and feedback! Thanks, guys!