pik-piam / mip

Model intercomparison visualization package
Other
1 stars 24 forks source link

droplevels in mipArea etc. drop deletedinfo attribute #67

Closed orichters closed 1 year ago

orichters commented 1 year ago

therefore, it does not show up anymore in the legend title

To reproduce:

q <- dplyr::filter(quitte::quitte_example_dataAR6, model == "GCAM")
q$identifier <- mip::identifierModelScen(q)
message(attr(q$identifier, "deletedinfo"))
q <- droplevels(q)
message(attr(q$identifier, "deletedinfo"))

That is for me a rather unexpected behavior and I would like to know how I can attach some piece of information here to a quitte object such that it is still available after going through droplevels in mip plotting scripts.

It seems to me that replacing droplevels(q) by droplevels(q, except = intersect(names(q), "identifier")) everywhere in the relevant mip functions should do the job. Any objections?

pfuehrlich-pik commented 1 year ago

I don't know about the specifics of droplevels in a mip or quitte context, but I know that R is by default dropping attributes after all kinds of operations, this is not specific to droplevels. The reasoning is that after modification of an object its attributes might no longer fit to the actual data. That being said I think this leads to lots of confusion and bugs, and I also certainly missed this and struggled quite a bit with lost attributes. You can read more about this in Advanced R.

Attributes should generally be thought of as ephemeral. For example, most attributes are lost by most operations:

attributes(a[1])
#> NULL
attributes(sum(a))
#> NULL
orichters commented 1 year ago

Ok, thanks, then I will implement it differently and add color.dim.name as already used here in mipLineHistorical to other plot functions and use that…

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q commented 1 year ago
  1. What @pfuehrlich-pik wrote.
  2. It seems to me that replacing droplevels(q) by droplevels(q, except = intersect(names(q), "identifier")) everywhere in the relevant mip functions should do the job.

    That's rather error-prone. For one thing, it will not be clear to people why that is there (triggering laborious git history searches), for another, people will not remember to include this whenever they add a droplevels() somewhere.

What is the actual problem you are trying to solve?