bp-kelley / descriptastorus

Descriptor computation(chemistry) and (optional) storage for machine learning
Other
220 stars 62 forks source link

Logging issue #8

Closed Nummi83 closed 1 year ago

Nummi83 commented 2 years ago

Importing MakeGenerator from descriptastorus.descriptors.DescriptorGenerator messes up how logging works. Below is an MWE demoing this unwanted behavior.

import logging

from descriptastorus.descriptors.DescriptorGenerator import MakeGenerator

logging.basicConfig(handlers=[logging.FileHandler("temp.log", "a", "utf-8"),
                              logging.StreamHandler()], level=logging.INFO,
                    format="%(asctime)s %(levelname)s [%(module)s.%(funcName)s]: %(message)s")

def status():
    logging.info("Hello!")

if __name__ == '__main__':
    status()

Running this code does not result in "Hello!" being written to the log file "temp.log" as is expected. If you comment out the row from descriptastorus.descriptors.DescriptorGenerator import MakeGenerator the log entry is written without error.

To verify this issue I created a new conda environment, first installing rdkit conda install -c rdkit rdkit, followed by pip install git+https://github.com/bp-kelley/descriptastorus and pip install scipy.

bp-kelley commented 2 years ago

The generator does "work" upon import which interfaces with logging. I'm in the process of moving logging to it's own namespace which will help somewhat, but your solution here is to configure logging before importing descriptastorus

import logging

logging.basicConfig(handlers=[logging.FileHandler("temp.log", "a", "utf-8"),
                              logging.StreamHandler()], level=logging.INFO,
                    format="%(asctime)s %(levelname)s [%(module)s.%(funcName)s]: %(message)s")

from descriptastorus.descriptors.DescriptorGenerator import MakeGenerator

def status():
    logging.info("Hello!")

if __name__ == '__main__':
    status()

Not the best solution but I think until I make the data loading "lazy" you will always need to do this.

bp-kelley commented 2 years ago

I'll consider making the data loading lazy in 3.0.0 but I kind of like the fast failure for descriptastorus right now.

bp-kelley commented 1 year ago

All the logging is now in the "descriptastorus" logger.

pip install descriptastorus>=2.6.1