_get_reader_kwargs is setting kwargs incorrectly.

[simonrp84]

Describe the bug
The function _get_reader_kwargs in satpy/satpy/readers/__init__.py is incorrectly setting the reader_kwargs in some circumstances. Rather than setting to reader_kwargs = {"reader_name": {}}, it instead returns a dict composed of the reader name.

To Reproduce

scn = Scene(some_files, some_reader)  # Note: No `reader_kwargs`

Stepping through the code with the debugger, I see this:

def _get_reader_kwargs(reader, reader_kwargs):

...

    reader_kwargs = reader_kwargs or {}

    # ensure one reader_kwargs per reader, None if not provided
    if reader is None:
        reader_kwargs = {None: reader_kwargs}
    elif reader_kwargs.keys() != set(reader):
        reader_kwargs = dict.fromkeys(reader, reader_kwargs)

...

    return (reader_kwargs, reader_kwargs_without_filter)

In the __init__.py file.
In the above example, reader is not None, but we're not passing any reader_kwargs to this function, so in the elif we end up setting reader_kwargs to `{'r': [], 'e': [], 'a': [], 'd': []} or whatever the reader is called.

As satpy seems to work OK this is probably not a big deal, but I presume it should be fixed!

[djhoese]

I'm a little confused by your calling structure. Scene should never be called without kwargs. Your example code is only positional arguments. We could update the Scene to only take keyword arguments and no positional arguments with the newer Python syntax my_func(pos1, pos2, *, kwarg1=None, kwarg2=None):.

[djhoese]

Ok maybe I shouldn't say "never", it looks like you are passing them in the right order. What exactly are you passing in your example code?

[djhoese]

I think more important is this function which gets called first (before _get_reader_kwargs):

satpy/satpy/readers/__init__.py

Lines 606 to 618 in ee2273a

	def _get_reader_and_filenames(reader, filenames):
	if reader is None and isinstance(filenames, dict):
	# filenames is a dictionary of reader_name -> filenames
	reader = list(filenames.keys())
	remaining_filenames = set(f for fl in filenames.values() for f in fl)
	elif reader and isinstance(filenames, dict):
	# filenames is a dictionary of reader_name -> filenames
	# but they only want one of the readers
	filenames = filenames[reader]
	remaining_filenames = set(filenames or [])
	else:
	remaining_filenames = set(filenames or [])
	return reader, filenames, remaining_filenames

If I understood your use on slack, you're passing filenames from find_files_and_readers so your filenames are probably {"some_reader": [list of files]}, right? If you're passing reader then (which is unnecessary given the filenames dictionary), then the elif in the above function is entered so filenames should be [list of files] and reader should still be "some_reader". This assumes that you are passing "some_reader" to the Scene as the reader name and not ["some_reader"] or something else. There is a very good chance your calling structure is not tested...but I find that hard to believe given the elif in the above code and our test coverage.

[simonrp84]

Quick comment as it's late here:
I first tried collecting files with find_files_and_readers but encountered an error. So I then simply passed the filename directly:

scn = Scene(['/my/dir/my_file.tif'], reader='oli_tirs_l1_tif')

• reader name here is what I'm working on

[djhoese]

I'm unable to reproduce this with what I assume would be equivalent:

In [2]: scn = Scene(["my.tif"], reader="generic_image")

In [3]: scn._readers
Out[3]: {'generic_image': <satpy.readers.yaml_reader.FileYAMLReader at 0x7d5987b38230>}

Same result if I remove the reader= portion of the call. Are you getting an error when you do this?