quaquel / EMAworkbench

workbench for performing exploratory modeling and analysis
BSD 3-Clause "New" or "Revised" License
128 stars 90 forks source link

change to log message and log level in feature scoring #318

Closed quaquel closed 12 months ago

quaquel commented 1 year ago

fixes #261

columns with the same value for all entries don't matter for feature scoring, so they can be ignored.

coveralls commented 1 year ago

Coverage Status

coverage: 80.206%. remained the same when pulling 1739302e24872d8248318201128dcd89689aa5f7 on fs_logmessage into b6ac5ccc4a6fa525979a17c98b981c58373e8e2d on master.

EwoutH commented 1 year ago

But it's still nice to know that they are ignored, right? If so, maybe they shouldn't be hidden in a debug log.

quaquel commented 1 year ago

I was debating this with myself as well. What annoys me in the current implementation is that you can get multiple messages about columns that are removed (e.g., policy, model). Moreover, in other places, we remove those columns silently anyway.

EwoutH commented 1 year ago

My instinct is to follow PEP 20 here:

Explicit is better than implicit.

So rather than removing it here, we might want to add it in other places that make sense.

That said, maybe some columns dropping are special cases: policy and scenario might be two of them. If you only have one policy (and test different scenarios / uncertainty), it might be logical that policy is dropped (so it's indented behaviour). In other cases, it might not.

So that's I think what my original issue was about: I got the message in #261, now what? It should either give something concrete to consider, or some call to action.

quaquel commented 1 year ago

It is not a call to action, hence my choice to lower the log-level. The original idea of the message was more a 'just so you know' type message. One way of solving the issue is to make this behavior clear in the documentation while also lowering the log level as I have now done.

EwoutH commented 1 year ago

Good point, but a “just so you know” message is exactly where the info level if for, right? The warn level is for actions to consider, and the debug to have as much information available as possible to find unexpected behaviour.

Are there any special cases we can threat to make it more clear?

(on a higher level, I’m fine with all options, since this is really minor)