vdWHandler 0.4 does not assume a default value of method in up-conversion

[mattwthompson]

Describe the bug

The logic I added in vdWHandler.__init__ (#1679, 0.14.2) assumed that the method kwarg was being passed, which usually was not the case. This results in a surprising error, surprising in the sense that I hadn't considered it. Passing the method kwarg through works fine.

This naturally breaks some workflows using the API, which some packages like Evaluator relied on. Loading OFFXMLs seems fine.

To Reproduce

In [1]: from openff.toolkit.typing.engines.smirnoff.parameters import vdWHandler

In [2]: vdWHandler(version=0.4).periodic_method, vdWHandler(version=0.4).nonperiodic_method
Out[2]: ('cutoff', 'no-cutoff')

In [3]: vdWHandler(version=0.3)
---------------------------------------------------------------------------
KeyError                                  Traceback (most recent call last)
Cell In[3], line 1
----> 1 vdWHandler(version=0.3)

File ~/software/openff-toolkit/openff/toolkit/typing/engines/smirnoff/parameters.py:2852, in vdWHandler.__init__(self, **kwargs)
   2843         logger.info(
   2844             'Successfully up-converted vdW section from 0.3 to 0.4. `method="cutoff"` '
   2845             'is now split into `periodic_method="cutoff"` '
   2846             'and `nonperiodic_method="no-cutoff"`.'
   2847         )
   2849     else:
   2850         raise NotImplementedError(
   2851             "Failed to up-convert vdW section from 0.3 to 0.4. Did not know "
-> 2852             f'how to up-convert `method="{kwargs["method"]}"`.'
   2853         )
   2854 super().__init__(**kwargs)

KeyError: 'method'

In [4]: vdWHandler(version=0.3, method="cutoff")
Out[4]: <openff.toolkit.typing.engines.smirnoff.parameters.vdWHandler at 0x16f95bf90>

[mattwthompson]

I'm confused how this is happening; the code looks the same as ElectrostaticsHandler but the behavior is different:

In [11]: ElectrostaticsHandler(version=0.3).version
Out[11]: <Version('0.4')>

In [12]: vdWHandler(version=0.3)
---------------------------------------------------------------------------
KeyError                                  Traceback (most recent call last)
Cell In[12], line 1
----> 1 vdWHandler(version=0.3)

File ~/software/openff-toolkit/openff/toolkit/typing/engines/smirnoff/parameters.py:2852, in vdWHandler.__init__(self, **kwargs)
   2843         logger.info(
   2844             'Successfully up-converted vdW section from 0.3 to 0.4. `method="cutoff"` '
   2845             'is now split into `periodic_method="cutoff"` '
   2846             'and `nonperiodic_method="no-cutoff"`.'
   2847         )
   2849     else:
   2850         raise NotImplementedError(
   2851             "Failed to up-convert vdW section from 0.3 to 0.4. Did not know "
-> 2852             f'how to up-convert `method="{kwargs["method"]}"`.'
   2853         )
   2854 super().__init__(**kwargs)

KeyError: 'method'

I thought the issue was handling KeyError by passing None to dict.pop, but that's done in both places.

[mattwthompson]

My mistake, this should be fixed in #1689 by

diff --git a/openff/toolkit/typing/engines/smirnoff/parameters.py b/openff/toolkit/typing/engines/smirnoff/parameters.py
index acf2d936..290d1628 100644
--- a/openff/toolkit/typing/engines/smirnoff/parameters.py
+++ b/openff/toolkit/typing/engines/smirnoff/parameters.py
@@ -2834,7 +2834,7 @@ class vdWHandler(_NonbondedHandler):
             logger.info("Attempting to up-convert vdW section from 0.3 to 0.4")
 
             # This is the only supported value of "method" is version 0.3
-            if kwargs.get("method") == "cutoff":
+            if kwargs.get("method") in ("cutoff", None):
                 kwargs["periodic_method"] = "cutoff"
                 kwargs["nonperiodic_method"] = "no-cutoff"
                 kwargs["version"] = 0.4