Bug: KeyError rather than TypeError in LocStream for python >=3.12

[xylar]

Requirements

• Reviewed ESMPy Documentation
• Searched GitHub Discussions

Affiliation(s)

LANL

ESMF Version

v8.7.0 (and earlier)

Issue

I'm trying to build ESMPy 8.7.0 on conda-forge:
conda-forge/esmpy-feedstock#85
In the tests, I am seeing a KeyError:

___________________________ TestLocStream.test_slice ___________________________

self = <esmpy.test.test_api.test_locstream.TestLocStream testMethod=test_slice>

    @pytest.mark.skipif(pet_count()!=1, reason="test must be run in serial")
    def test_slice(self):
        locstream = LocStream(5, name="Test LocStream")
    
>       locstream2 = locstream[0:2]

src/esmpy/test/test_api/test_locstream.py:95: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = LocStream:
    name = 'Test LocStream' 
    lower_bounds = array([0], dtype=int32) 
    upper_bounds = array([5], dtype=int32) 
    keys = dict_items([]) 

slc = slice(0, 2, None)

    def __getitem__(self, slc):
        # initialize slc_ls
        slc_ls = slc
    
        # check that this is actually a slicing operation and not just regular item retrieval
        if not isinstance(slc, str):
            # parallel slicing is not yet enabled (collective operation)
            if pet_count() > 1:
                raise SerialMethod
            # re-initialize slc_ls
            slc_ls = get_formatted_slice(slc, self.rank)
    
        # slice at will
        try:
>           ret = super(LocStream, self).__getitem__(slc_ls)
E           KeyError: slice(0, 2, None)

src/esmpy/api/locstream.py:130: KeyError

The cause of this is that slice() objects are hashable in Python >=3.12, whereas they were not in Python <3.12, and raised a TypeError. I believe the fix would be to change the try/catch to:

       # slice at will
       if not isinstance(slc_ls, slice):
           ret = super(LocStream, self).__getitem__(slc_ls)
       else:
           # as of Python 3.12, slice objects are hashable so don't
           # raise a TypeError
           ret = self.copy()
...

This at least seems to have allowed all tests to pass.

Autotag

@billsacks

[xylar]

In general, it seems like you all should probably either be testing with python 3.12 and 3.13 or should explicitly constrain python to be below these versions.

[billsacks]

Thank you for reporting this @xylar . I'll take a look soon, and will also review the python versions that we're testing with.

[billsacks]

@xylar - sorry it took me a while to get to this, but I have now brought in your suggested change: 338cd06. I looked this over pretty carefully and agree with the change you proposed. As noted in that commit, I think there are a number of possible cases where the new code will behave differently from the old code, but as far as I can tell, all of those cases would have raised exceptions in the old code, and now will just raise different exceptions, so I'm not too concerned.

Part of what held me up was that I ran into different errors while testing with python 3.12 on my Mac: I was getting Segmentation Faults in two places. I also fixed this issue today: 6d5fdfe.

I will now add nightly testing with python3.12 and python3.13.

Thank you again for reporting this and for providing the fix!

[xylar]

Thanks @billsacks!

[billsacks]

@xylar - also FYI just in case you run into this: In updating our ESMPy test environments to test more recent versions of python as well as other packages, I ran into an issue with recent versions of numpy: #329. I'll work on a fix for that as well, so that we can hopefully have support for the latest python & numpy versions in the upcoming 8.8 release.

[xylar]

That sounds good. And it sounds like maybe it would be good to set constraints on esmpy dependencies to explicitly exclude numpy 3 and python 3.14 until they are tested, based on this experience.

[billsacks]

Good idea. You'll have to excuse my ignorance, though: Would the best way to do this be via https://github.com/esmf-org/esmf/blob/develop/src/addon/esmpy/pyproject.toml?

[billsacks]

(I'm particularly wondering if that will be sufficient for constraints on the conda side, or if something else would need to be done in addition to that.)

[xylar]

Yes, exactly. We use that file as our instructions from you the developers when we updated the conda package.

Ideally, there would be an upper bound on python on line 10 and both lower and upper bounds numpy on line 13.