aphalo / ggpp

Grammar of graphics extensions to 'ggplot2'
122 stars 10 forks source link

How to deal with the 'ggplot2' 3.5.0 update? #51

Closed aphalo closed 5 months ago

aphalo commented 7 months ago

NPC translation functions to support named positions

With 'ggplot2' 3.5.0, support is built-in, but with a different syntax, making the _npc geoms from 'ggpp' rather redundant. The changes in 'ggplot2', however, do not break the current _npc geoms. On the other hand, getting rid of the pseudo aesthetics npcx and npcy would simplify maintenance.

I have become used to setting positions based on words and allowing this would easy the transition from the 'ggpp' syntax to the new syntax supported natively by 'ggplot2'.

There are several possible approaches:

  1. as.npc() could be used as x = as.npc("left") this could use other names, or in fact could use the existing functions for justification This could be a pull resquest to 'ggplot2'.
  2. aes_npc()
  3. npcx() and npcy()
  4. Rewrite the existing _npc geoms as wrappers using the new syntax.
aphalo commented 7 months ago

At this moment I think that the smoother switch would be to implement both 1. and 4. from the list above. But implementing 4., means making 'ggplot2' 3.5.0 a requirement... how long to wait?

aphalo commented 7 months ago

@danielinteractive Please look at the question below (more timely) and the question above (requires more thought, I think).

Enhanced geom_label() supports rotation with angle

This is implemented in 'ggpp' but to get it to work with 'ggplot2' pre 3.5.0, I needed to copy several functions from 'ggplot2' 3.5.0 into 'ggpp'. This seems to me rather fragile.

Enhancements keys and legends

I have today implemented proper drawkey functions in 'ggpp', using 'ggplot2' 3.5.0 as a model.

'ggpp' in branch develop-0.5.7

It depends on 'ggplot2' 3.5.0 and passes all tests, at least locally. 'ggpp' in branch master fails one test with 'ggplot2' 3.5.0, failure is in one unit test that fails to correctly draw the key/legend under some conditions. I have raised an issue about this in 'ggplot2'. Another issue I raised last week in 'ggplot2', was quickly addressed so 'ggplot2' installed from GitHub gets this fixed.

When to start depending on 'ggplot2' 3.5.0?

Dear Daniel, I would like your opinion on this: when do you think that making 'ggpp' depend on 'ggplot' >= 0 3.5.0 will be o.k. for 'ggplot.utils'? The geoms for inset plots and tables work under 'ggplot2' 3.5.0 without any change to the code or plots produced. There is one bug fixed that affected geom_point_s() in the master branch. This is the only significant change ahead of the CRAN release. It is available through the CRAN-like repo at https://aphalo.r-universe.dev/. I have done all other edits in the develop-0.5.7 branch, and will mostly continue new development in it.

aphalo commented 7 months ago
  1. Implemented as_npc(), as_npcx() and as_npcy() in 'ggpp'. (With minimal new code.)
danielinteractive commented 7 months ago

Dear Pedro @aphalo , thanks for reaching out! So looking at the ggplot2.utils imports here, we only use geom_table() and geom_table_npc() alongside several themes from ggpp. So from reading above we could already depend on the latest ggplot2 version now?

aphalo commented 7 months ago

Dear Daniel, If depending on 'ggplot2' 3.5.0 is no problem for your users, then, yes. I would expect 'ggplot2' 3.5.1 to be released rather soon with some rough edges smoothed out. I will continue the development of 'ggpp' without trying to remain backwards compatible with older 'ggplot2' versions, but most likely submit the new version to CRAN after one or two months. I will let you know before I submit it. Thanks!

aphalo commented 5 months ago

Dear Daniel @danielinteractive, I just noticed that the approach using I() for NPC as implemented in 'ggplot2' 3.5.0 and 3.5.0.9000 supports only the case when the variable mapped to x or y is continuous. It does not work when factors are mapped, a case that 'ggpp' geoms support. So, even if I make the next version of 'ggpp' depend on 'ggplot2' 3.5.0, because of the legends, I will keep the implementation of geom_table_npc() and the other _npc geoms unchanged. I have fixed a few bugs in the meantime, so I will submit the new version to CRAN rather soon, but only after I solve another couple of open issues. (There is a function without unit tests because I have to rewrite it. I will add unit tests before submission to CRAN.)