rl-institut / oemof-B3

An open-source energy system model for Brandenburg/Berlin.
https://oemof-b3.readthedocs.io/
GNU Affero General Public License v3.0
9 stars 5 forks source link

Logger in data_processing.py #273

Closed monika-o closed 1 year ago

monika-o commented 2 years ago

This little pull request introduces logging to tools/data_processing.py (see #258).

monika-o commented 2 years ago

I've made the changes similar to the logger implementation in scripts/build_datapackage.py. I tried to test it by running the scenario 2050-100-el_eff up to the preprocessing step. This didn't produce an error, but I didn't see any of the statements in the logfile (2050-100-el_eff.log), so I am not sure whether the logging didn't work or whether it just hasn't been called. @jnnr Do you have an idea how to test it, or are the changes so small and clear that we can just merge?

jnnr commented 2 years ago

Thank you! The location of the logfiles has changed with #247 to save the logfiles next to the results. I would check if the files logging the data processing steps can be found in results/_resources. Maybe build_datapackage does not use any of the functions that have logging now, so you don't see it?

monika-o commented 2 years ago

I think I have found the correct logfile, but I've seen now that there are some lines that I have overlooked. I will notify you when I am done.

monika-o commented 2 years ago

Here is an update: Some of the logging statements are indeed called when testing it, and they are printed on the terminal. The problem is that they are not saved to the logfile (2050-100-el_eff.log, and also not to any other logfile). I am now trying to figure out the reason for this and would be glad about your ideas if you have some.

monika-o commented 2 years ago

The reason is that the functions in data_processing.py use <RootLogger root (INFO)> as logger, whereas the script build_datapackage.py uses the local logger <Logger build_datapackage (INFO)> (and the same issue most probably occurs with optimize.py).

monika-o commented 1 year ago

I see two ways to solve this:

  1. Simply passing the logger as a parameter to the function (in a total of 6 functions in which we use logging) or
  2. Creating a new logger for data processing. This logger, however, must know the location of the logfile (i.e. the scenario) and I didn't find out yet how to access that information.
jnnr commented 1 year ago

Thanks for your approach. Looks good, but will break if the naming convention of naming the logger same as the script is not obeyed.

I found this: https://stackoverflow.com/questions/26405841/python-logging-from-library-module

jnnr commented 1 year ago

Hi @monika-o, I saw the usage of logging in another module today. Maybe this approach works? https://github.com/rl-institut/oemoflex/blob/9f0563425e7ac6450a537c1128d8b33d60d716bc/oemoflex/postprocessing/core.py

henhuy commented 1 year ago

Wasn't as easy as I expected. Now logger is added in data_preprocessing.py. Name of logfile is read from input parameters and MUST end with ".log" to be detected.

One off-topic commit: Had to fix pre-commit tag (master/main not possible any more - must be tagged to specific version)

henhuy commented 1 year ago

Thanks for all your work for this, @monika-o and @henhuy!

I did not expect it to be a problem that uncommon to lead to so many different workarounds.

To put an end to this, I would like to accept your solution, @henhuy. The only difference to what I originally had in mind is that the logger in dataprocessing has the category "data_processing" and does not state the script from which it is called. But I am ok with it.

Oh no. Had this implemented at one point, but thought it would be better to see module name... :( But as this issue already took so long, I also would close it here...

monika-o commented 1 year ago

I would also be fine with it if you close this pull request now.