search

pik-piam / gdx

R package | readGDX package for R
Other
1 stars 4 forks source link

chage default to readGDX(react = 'silent')? #1

Closed 0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q closed 4 years ago

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q commented 4 years ago

How about changing the default of parameter react in readGDX() from warning to silent?

https://github.com/pik-piam/gdx/blob/1f3c0c5a06fea30014ecbca3336e1e6d232fe0c5/R/readGDX.R#L99

Little code-census: searching through calls to readGDX() in all pik-piam packages:

react count
<none> 1151
'silent' 107
'warning' 8
'error' 0
'quiet' 5
F 2

Most users don't seem to care too much whether objects are found or not, and of those who seem to care, most are actually ok with silent, and some don't care enough to put down a legal parameter.

Of the first group (not caring or ok with silent), 544 calls are in the remind package, which elicits a truck load of warnings about 

<some_parameter> not found in GDX! <some_parameter2> returned

due to all the renaming that has been going on.

This makes warnings quite meaningless, because a) this is designed-for behaviour and b) it also makes it impossible to see any other warnings, since R only keeps 50 of them.

So how about changing the default to silent, cutting down on all those warnings?

tscheypidi commented 4 years ago

Well, the idea warning being the default is to identify cases in which readGDX fails to read an object and no measures are taken in the code to deal with it. As soon as someone acknowledges the possibility that no data is read in and deals with it in the code, this person should switch to react="silent". I suggest to inform developer, who are currently not aware of the react argument or how to properly use it, how the argument should be used.

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q commented 4 years ago

Let me rephrase: it's not about whether objects are found or not, its about the actual name under which objects are found -- people don't care about that.

So how about not issuing a warning when readGDX() works as designed by reading an object that can have one of multiple names, depending on the vintage of the .gdx file being read?

Example:

c_damage                <- readGDX(gdx,"cm_damage","c_damage",format="first_found")
Warning message:
In readGDX(gdx, "cm_damage", "c_damage", format = "first_found") :
  cm_damage not found in GDX! c_damage returned

Would cut down on a lot of warning() spam in the remind package at least.

tscheypidi commented 4 years ago

ok, that's another story. In that case we would need to ignore the react-argument for case of multiple names given. Switching the default of react would also silence warnings in cases in which no object at all is found.

So my proposal would be: Keep the default for react, but remove the warning in cases in which not the first but instead an alternative name is found in the gdx. Does that makes sense?

0UmfHxcvx5J7JoaOhFSs5mncnisTJJ6q commented 4 years ago

So my proposal would be: Keep the default for react, but remove the warning in cases in which not the first but instead an alternative name is found in the gdx. Does that makes sense?

Very much so. You want to do it, or should I?

tscheypidi commented 4 years ago

I have just updated the gdx package. From version 1.51.0 onwards the warning should not show up again.

Thanks for your suggestion!