code review feedback

qcodes/instrument/visa.py

@@ -27,7 +27,9 @@ def _close_visa_handle(
     handle: pyvisa.resources.MessageBasedResource, name: str
 ) -> None:
     log.info(
-        f"Closing VISA handle to {name} as there are no non weak references to the instrument."
+        "Closing VISA handle to %s as there are no non weak "
+        "references to the instrument.",
+        name,
     )
     handle.close()
 
@@ -114,6 +116,7 @@ def __init__(
                 )
         else:
             visa_handle, visabackend = self._connect_and_handle_error(address, visalib)
+        finalize(self, _close_visa_handle, visa_handle, str(self.name))
 
         self.visabackend: str = visabackend
         self.visa_handle: pyvisa.resources.MessageBasedResource = visa_handle
@@ -128,7 +131,7 @@ def __init__(
 
         self.set_terminator(terminator)
         self.timeout.set(timeout)
-        finalize(self, _close_visa_handle, self.visa_handle, str(self.name))
+
 
     def _connect_and_handle_error(
         self, address: str, visalib: str | None

qcodes/tests/test_instrument.py

@@ -434,10 +434,10 @@ def use_some_instrument() -> None:
         _ = Instrument("SomeInstrument")
         assert list(Instrument._all_instruments.keys()) == ["SomeInstrument"]
 
-    assert list(Instrument._all_instruments.keys()) == []
+    assert len(Instrument._all_instruments) == 0
     use_some_instrument()
     gc.collect()
-    assert list(Instrument._all_instruments.keys()) == []
+    assert len(Instrument._all_instruments) == 0
 
 def test_snapshot_and_meta_attrs() -> None:
     """Test snapshot of InstrumentBase contains _meta_attrs attributes"""

qcodes/tests/test_visa.py

@@ -129,13 +129,13 @@ def use_magnet():
         use_magnet()
         gc.collect()
     # at this stage the instrument created in use_magnet has gone out of scope
-    # and we have triggered an explicit gc so the weakref.finalize function has been triggered
-    # We test this
+    # and we have triggered an explicit gc so the weakref.finalize function
+    # has been triggered. We test this
     # and the instrument should no longer be in the instrument registry
-    assert list(Instrument._all_instruments.keys()) == []
+    assert len(Instrument._all_instruments) == 0
     assert (
-        caplog.records[-1].message
-        == "Closing VISA handle to x as there are no non weak references to the instrument."
+        caplog.records[-1].message == "Closing VISA handle to x as there are no non "
+        "weak references to the instrument."
     )