Merge pull request #5953 from johnhaddon/deprecatePlugValueWidgetUpdate

PlugValueWidget : Emit `DeprecationWarning` for deprecated methods

Changes.md

@@ -29,6 +29,9 @@ API
   - Added `settings()` method, which returns a node hosting plugs specifying settings for the editor.
   - Added `_updateFromSettings()` method, which is called when a subclass should update to reflect changes to the settings.
 - SceneEditor : Added new base class to simplify the creation of scene-specific editors.
+- PlugValueWidget :
+  - A `DeprecationWarning` is now emitted for any subclasses still implementing the legacy `_updateFromPlug()` or `_updateFromPlugs()` methods. Implement `_updateFromValues()`, `_updateFromMetadata()` and `_updateFromEditable()` instead.
+  - A `DeprecationWarning` is now emitted by `_plugConnections()`. Use `_blockedUpdateFromValues()` instead.
 
 Breaking Changes
 ----------------

python/GafferImageUI/ImageViewUI.py

@@ -596,11 +596,11 @@ def __updateInBackgroundPostCall( self, backgroundResult ) :
 			#
 			# - This widget being hidden.
 			# - A graph edit that will affect the image and will have
-			#   triggered a call to _updateFromPlug().
-			# - A graph edit that won't trigger a call to _updateFromPlug().
+			#   triggered a call to `__plugDirtied()`.
+			# - A graph edit that won't trigger a call to `__plugDirtied()`.
 			#
 			# LazyMethod takes care of all this for us. If we're hidden,
-			# it waits till we're visible. If `updateFromPlug()` has already
+			# it waits till we're visible. If `__plugDirtied()` has already
 			# called `__updateLazily()`, our call will just replace the
 			# pending call.
 			self.__updateLazily()

python/GafferUI/PlugValueWidget.py

@@ -365,8 +365,7 @@ def _editable( self, canEditAnimation = False ) :
 	## \deprecated
 	def _plugConnections( self ) :
 
-		## \todo Emit DeprecationWarning once we can reasonably expect
-		# Gaffer `1.1.x.x` to be history.
+		warnings.warn( "`PlugValueWidget._plugConnections()` is deprecated. Use `_blockedUpdateFromValues()` instead", DeprecationWarning, 2 )
 
 		return (
 			self.__plugDirtiedConnections +
@@ -517,16 +516,12 @@ def __callLegacyUpdateMethods( self ) :
 		# `_updateFromMetadata()`, `_updateFromValues()` etc. But we still
 		# support calling them if they exist.
 
-		updateMethod = getattr( self, "_updateFromPlugs", None )
-		if updateMethod is None :
-			updateMethod = getattr( self, "_updateFromPlug", None )
-
-		if updateMethod is not None :
-			## \todo Emit DeprecationWarning. It doesn't make sense to do
-			# this until we can reasonably expect Gaffer `1.1.x.x` to be
-			# out of use, as maintaining a subclass to use both the old
-			# and the new API would be incredibly painful.
-			updateMethod()
+		for methodName in ( "_updateFromPlugs", "_updateFromPlug" ) :
+			updateMethod = getattr( self, methodName, None )
+			if updateMethod is not None :
+				warnings.warn( f"`PlugValueWidget.{methodName}()` is deprecated. Implement `_updateFromValues()`, `_updateFromMetadata()` and `_updateFromEditable()` instead", DeprecationWarning, 2 )
+				updateMethod()
+				return
 
 	@GafferUI.LazyMethod()
 	def __callUpdateFromValues( self ) :

python/GafferUITest/PlugValueWidgetTest.py

@@ -36,6 +36,7 @@
 ##########################################################################
 
 import unittest
+import warnings
 import weakref
 
 import IECore
@@ -331,36 +332,40 @@ def testLegacyUpdates( self ) :
 		script = Gaffer.ScriptNode()
 		script["add"] = GafferTest.AddNode()
 
-		# Should do one update during construction.
-		widget = self.LegacyUpdateCountPlugValueWidget( script["add"]["op1"] )
-		self.assertEqual( widget.updateCount, 1 )
-		self.assertTrue( widget.updateContexts[0].isSame( script.context() ) )
+		with warnings.catch_warnings() :
 
-		# And shouldn't update when the context changes
-		# because the value is static.
-		script.context().setFrame( 2 )
-		self.assertEqual( widget.updateCount, 1 )
+			warnings.simplefilter( "ignore", DeprecationWarning )
 
-		# Changing the plug should cause an update.
-		widget.setPlug( script["add"]["op2"] )
-		self.assertEqual( widget.updateCount, 2 )
-		self.assertTrue( widget.updateContexts[1].isSame( script.context() ) )
+			# Should do one update during construction.
+			widget = self.LegacyUpdateCountPlugValueWidget( script["add"]["op1"] )
+			self.assertEqual( widget.updateCount, 1 )
+			self.assertTrue( widget.updateContexts[0].isSame( script.context() ) )
 
-		# But the value is still static, so changing the
-		# context should have no effect.
-		script.context().setFrame( 3 )
-		self.assertEqual( widget.updateCount, 2 )
+			# And shouldn't update when the context changes
+			# because the value is static.
+			script.context().setFrame( 2 )
+			self.assertEqual( widget.updateCount, 1 )
 
-		# Changing the plug again should cause an update again.
-		widget.setPlug( script["add"]["sum"] )
-		self.assertEqual( widget.updateCount, 3 )
-		self.assertTrue( widget.updateContexts[2].isSame( script.context() ) )
+			# Changing the plug should cause an update.
+			widget.setPlug( script["add"]["op2"] )
+			self.assertEqual( widget.updateCount, 2 )
+			self.assertTrue( widget.updateContexts[1].isSame( script.context() ) )
 
-		# And now changing the context does cause an update, because
-		# the plug's value is computed.
-		script.context().setFrame( 4 )
-		self.assertEqual( widget.updateCount, 4 )
-		self.assertTrue( widget.updateContexts[3].isSame( script.context() ) )
+			# But the value is still static, so changing the
+			# context should have no effect.
+			script.context().setFrame( 3 )
+			self.assertEqual( widget.updateCount, 2 )
+
+			# Changing the plug again should cause an update again.
+			widget.setPlug( script["add"]["sum"] )
+			self.assertEqual( widget.updateCount, 3 )
+			self.assertTrue( widget.updateContexts[2].isSame( script.context() ) )
+
+			# And now changing the context does cause an update, because
+			# the plug's value is computed.
+			script.context().setFrame( 4 )
+			self.assertEqual( widget.updateCount, 4 )
+			self.assertTrue( widget.updateContexts[3].isSame( script.context() ) )
 
 	def tearDown( self ) :