Skip to content

Conversation

@pchelle
Copy link
Collaborator

@pchelle pchelle commented Oct 22, 2025

As suggested in this ggplot2 PR

@pchelle pchelle requested review from Felixmil and Yuri05 October 22, 2025 16:16
@pchelle
Copy link
Collaborator Author

pchelle commented Oct 22, 2025

@KatrinCoboeken is this already accounted for in {opsuite.plots} ?

@pchelle
Copy link
Collaborator Author

pchelle commented Oct 22, 2025

@KatrinCoboeken is this already accounted for in {opsuite.plots} ?

I checked, you actually use geom_text so the fix is not relevant for the {opsuite.plots} package

Copy link
Contributor

@Felixmil Felixmil left a comment

Choose a reason for hiding this comment

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

Snapshots need to be updated too.

@KatrinCoboeken
Copy link
Collaborator

Yes, in {ospsuite.plots} I also had to update the watermark feature with ggplot2 version 4. But it is done.

Copy link
Contributor

@Felixmil Felixmil left a comment

Choose a reason for hiding this comment

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

Plots look the same, the changes are svg "internal". All good.

@Felixmil
Copy link
Contributor

Should we expect plot changes in other packages ?

@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 84.24%. Comparing base (ad57bbb) to head (511f078).
⚠️ Report is 4 commits behind head on develop.

Files with missing lines Patch % Lines
R/utilities-background.R 50.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##           develop     #537       +/-   ##
============================================
+ Coverage    71.26%   84.24%   +12.98%     
============================================
  Files           63       63               
  Lines         4326    11033     +6707     
============================================
+ Hits          3083     9295     +6212     
- Misses        1243     1738      +495     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@pchelle
Copy link
Collaborator Author

pchelle commented Oct 23, 2025

Should we expect plot changes in other packages ?

This update should not change the plots in other packages.
Before the ggplot2 v4, annotation_custom() was not sensitive to scales functions, using this PR it simply remains unsensitive to scales functions

@Yuri05 Yuri05 requested a review from Copilot October 23, 2025 08:32
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes issue #536 by making watermark positioning independent from plot scales, following the approach suggested in ggplot2 PR #6182. The fix wraps the infinite boundary values with I() to prevent them from being affected by scale transformations.

Key Changes:

  • Wrapped annotation_custom() boundary parameters with I() to ensure scale-independent positioning
  • Fixed a typo in layer access (layerlayers)

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@Yuri05 Yuri05 merged commit 25a7320 into Open-Systems-Pharmacology:develop Oct 23, 2025
4 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.

5 participants