BUNPC / pysnirf2

Python package for reading, writing and validating Shared Near Infrared Spectroscopy Format (SNIRF) files
GNU General Public License v3.0
13 stars 12 forks source link

Setting `logging.basicConfig` causes exception on save() #37

Open zEdS15B3GCwq opened 1 year ago

zEdS15B3GCwq commented 1 year ago

Hi,

Windows 11 Python 3.11.4 snirf 0.7.4

I'm learning to use the module to create snirf files from scratch. I've run into a problem, where my code also uses the logging module and sets logging.basicConfig, and it results in a TypeError exception being thrown on save(). Minimal example follows:

import logging
from snirf import Snirf

logging.basicConfig(level=logging.DEBUG)

with Snirf("test.snirf", "w") as f:
    f.nirs.appendGroup()
    f.save()

Running this code results in:

INFO:root:Loading from file test.snirf
INFO:root:IndexedGroup Nirs at / in test.snirf initalized with 0 instances of <class 'snirf.pysnirf2.NirsElement'>
INFO:root:IndexedGroup Data at //nirs1 in None initalized with 0 instances of <class 'snirf.pysnirf2.DataElement'>
INFO:root:IndexedGroup Stim at //nirs1 in None initalized with 0 instances of <class 'snirf.pysnirf2.StimElement'>
INFO:root:IndexedGroup Aux at //nirs1 in None initalized with 0 instances of <class 'snirf.pysnirf2.AuxElement'>
INFO:root:1 th <class 'snirf.pysnirf2.NirsElement'> appended to IndexedGroup Nirs at / in test.snirf
--- Logging error ---
Traceback (most recent call last):
  File "C:\Users\dev\.pyenv\pyenv-win\versions\3.11.4\Lib\logging\__init__.py", line 1110, in emit
    msg = self.format(record)
          ^^^^^^^^^^^^^^^^^^^
  File "C:\Users\dev\.pyenv\pyenv-win\versions\3.11.4\Lib\logging\__init__.py", line 953, in format
    return fmt.format(record)
           ^^^^^^^^^^^^^^^^^^
  File "C:\Users\dev\.pyenv\pyenv-win\versions\3.11.4\Lib\logging\__init__.py", line 687, in format
    record.message = record.getMessage()
                     ^^^^^^^^^^^^^^^^^^^
  File "C:\Users\dev\.pyenv\pyenv-win\versions\3.11.4\Lib\logging\__init__.py", line 377, in getMessage
    msg = msg % self.args
          ~~~~^~~~~~~~~~~
TypeError: not all arguments converted during string formatting
Call stack:
  File "C:\Code\snirf_logging_bug.py", line 9, in <module>
    f.save()
  File "C:\Users\dev\Virtualenvs\snirfbug\Lib\site-packages\snirf\pysnirf2.py", line 6117, in save
    self._save(self._h.file)
  File "C:\Users\dev\Virtualenvs\snirfbug\Lib\site-packages\snirf\pysnirf2.py", line 5879, in _save
    self.nirs._save(*args)
  File "C:\Users\dev\Virtualenvs\snirfbug\Lib\site-packages\snirf\pysnirf2.py", line 1361, in _save
    self._order_names(h=h)  # Enforce order in the group names
  File "C:\Users\dev\Virtualenvs\snirfbug\Lib\site-packages\snirf\pysnirf2.py", line 1280, in _order_names
    self._cfg.logger.info(
Message: '//nirs1'
Arguments: ('--->', '//nirs')
INFO:root:Closing Snirf file test.snirf

No exception is thrown if the code only changes a property of the snirf root (e.g. formatVersion), but doesn't append a nirs group. Removing the logging.basicConfig line also removes the error.

zEdS15B3GCwq commented 1 year ago

As a workaround, it is possible to avoid invoking basicConfig at all, e.g. the below code sets various handlers and formatting, and works:

import logging
from snirf import Snirf

fmt = logging.Formatter(
    fmt="%(asctime)s,%(msecs)d %(levelname)-8s [%(filename)s:%(lineno)d] %(message)s",
    datefmt="%Y-%m-%d:%H:%M:%S",
)
_filelogger = logging.FileHandler("test.log")
_filelogger.setLevel(logging.DEBUG)
_filelogger.setFormatter(fmt)
_streamlogger = logging.StreamHandler()
_streamlogger.setLevel(logging.WARNING)
_streamlogger.setFormatter(fmt)

log = logging.getLogger(__name__)
log.setLevel(logging.DEBUG)
log.addHandler(_filelogger)
log.addHandler(_streamlogger)

with Snirf("test.snirf", "w") as f:
    f.nirs.appendGroup()
    f.save()

log.debug("debug")
log.warning("warning")

Nevertheless, I think it's still better to fix the cause of the exception.

sstucker commented 1 year ago

Hi, thanks for this--sorry to see I have misused the logging module. I am not actively maintaining this project and will take awhile to get to this fix. I can help you integrate the changes with the generative code if you make the necessary changes to the header.py

zEdS15B3GCwq commented 1 year ago

Thanks for the response. I'd love to contribute, but right now I'm too busy elsewhere, sorry. With the above workaround, my code works fine, and perhaps others who bump into the same issue can learn from it too. I hope you find the time to work on it one day.

ptrhbt commented 2 months ago

It seems to be fixed in: https://github.com/BUNPC/pysnirf2/commit/74242b74caf203f9c5ec44fb64281c2f88acf7a6

Is there a chance to trigger new official release?

rob-luke commented 1 month ago

I will try and move through a release now/in the coming days @ptrhbt

sstucker commented 1 month ago

Hopefully this is fixed in the new release but I will await confirmation