[Bug]: stacklevel=2 makes all warnings look like they're coming from docval

[sneakers-the-rat]

What happened?

Trying to figure out how namespace caching works, loaded a NWB file using an old version of NWB, and got some warnings about a cached namespace already being loaded, but they were being raised from the docval generator.

.venv/lib/python3.11/site-packages/hdmf/utils.py:668: UserWarning: Ignoring cached namespace 'hdmf-common' version 1.5.0 because version 1.8.0 is already loaded.
  return func(args[0], **pargs)
...

I think basically all of these warnings for methods/functions that are wrapped in docval should be stacklevel=1 so it's possible to find where they're being raised:

eg.

.venv/lib/python3.11/site-packages/hdmf/spec/namespace.py:531: UserWarning: Ignoring cached namespace 'core' version 2.4.0 because version 2.7.0 is already loaded.
  warn("Ignoring cached namespace '%s' version %s because version %s is already loaded."

Setting stacklevel>1 is for when the warning is raised in the body of a wrapping function itself

Steps to Reproduce

# using, eg. https://dandiarchive.org/dandiset/000025/draft/files
import pynwb
file = '001_140709EXP_A1.nwb'
nwbfile = pynwb.NWBHDF5IO(file, 'r')

Traceback

(above)

Operating System

macOS

Python Executable

Python

Python Version

3.11

Package Versions

pynwb: 2.8.1
hdmf: 3.14.3

[rly]

Thanks for the issue @sneakers-the-rat . I agree that that particular warning is at the wrong place and should probably have stacklevel=1. However, for some other cases, I think it should be whatever stacklevel gets to user code instead. A stacklevel=1 just shows the line that says warn(...) which is redundant. IMO it is useful for the user to know which line out of their script triggered the warning. However, a stacklevel=1 may be useful for developers who want to trace the stack to understand why the warning is being raised. They can still do this by first calling warnings.simplefilter('error') or otherwise changing the behavior or the warnings module. One best practice checker considers this as well.

For the cached namespace warning, with stacklevel=1, the warning shows:

/Users/rly/Documents/NWB/hdmf/src/hdmf/spec/namespace.py:531: UserWarning: Ignoring cached namespace 'hdmf-common' version 1.1.3 because version 1.8.0 is already loaded.
  warn("Ignoring cached namespace '%s' version %s because version %s is already loaded."

With stacklevel=3, the warning shows:

/Users/rly/Documents/NWB/hdmf/src/hdmf/build/manager.py:482: UserWarning: Ignoring cached namespace 'hdmf-common' version 1.1.3 because version 1.8.0 is already loaded.
  deps = self.__ns_catalog.load_namespaces(**kwargs)

Neither is very useful. There is no good way to show the line in user code (e.g., io = NWBHDF5IO(...)) that triggered the warning. So for cases where we can't get to user code reliably and docval is involved, we could set the stacklevel to 3+ or revert it to the default 1. I lean toward the default 1 for convention.

But for other cases where the user calls the function more directly, e.g.,

hdmf/src/hdmf/container.py

Lines 890 to 898 in a98e5e9

	def set_dataio(self, **kwargs):
	"""
	Apply DataIO object to the data held by this Data object
	"""
	warn(
	"Data.set_dataio() is deprecated. Please use Data.set_data_io() instead.",
	DeprecationWarning,
	stacklevel=2,
	)

hdmf/src/hdmf/backends/hdf5/h5_utils.py

Line 527 in a98e5e9

warnings.warn('Compression disabled by compression=False setting. ' +

I think the stacklevel should be 3 (or whatever it takes to get to user code).

related: #745 #1027

---

Related tangent

Showing lines outside of HDMF is possible with the new skip_file_prefixes keyword argument for warnings.warn introduced in Python 3.12 (ref) or by customizing the warnings module.

With skip_file_prefixes=(str(Path(__file__).parent.parent),)), the warning shows:

/Users/rly/Documents/NWB/pynwb/src/pynwb/__init__.py:309: UserWarning: Ignoring cached namespace 'hdmf-common' version 1.1.3 because version 1.8.0 is already loaded.
  super().load_namespaces(tm, path, file=file_obj, driver=driver, aws_region=aws_region)

Now the warning is showing a line from pynwb. Still not ideal, and while we could also skip pynwb file prefixes, we should not add references to pynwb in hdmf.

[sneakers-the-rat]

So for cases where we can't get to user code reliably and docval is involved, we could set the stacklevel to 3+ or revert it to the default 1. I lean toward the default 1 for convention.

But for other cases where the user calls the function more directly, e.g., I think the stacklevel should be 3 (or whatever it takes to get to user code).

agreed :) you know better than I which functions/methods are called directly or not!

skip_file_prefixes

ooooo i hadn't seen that that's super useful.