fixup! IECoreUSD : Option for relative prototype paths inside a PointInstancer

- Rename `gafferUSDPointInstancers...` variable to reflect that it is static and doesn't reference a Gaffer environment variable.
- Reduce nesting by dealing with both absolute cases in one block.
- Fix snake-casing of environment variables (extra '_' between words).
- Improve tests
	- Set environment variable for absolute test, in case the original environment contains the variable.
	- Add test for the case where the variable is not set.
	- Use `subTest` to consolidate testing code into only two functions.

Author: johnhaddon

contrib/IECoreUSD/src/IECoreUSD/PointInstancerAlgo.cpp

@@ -121,7 +121,7 @@ IECore::ObjectPtr readPointInstancer( pxr::UsdGeomPointInstancer &pointInstancer
 
 	// Prototype paths
 
-	const static bool gafferUSDPointInstancersRelativeProtoypes = checkEnvFlag( "IECOREUSD_POINTINSTANCER_RELATIVEPROTOTYPES", false );
+	const static bool g_relativePrototypes = checkEnvFlag( "IECOREUSD_POINTINSTANCER_RELATIVE_PROTOTYPES", false );
 
 	pxr::SdfPathVector targets;
 	Canceller::check( canceller );
@@ -134,24 +134,17 @@ IECore::ObjectPtr readPointInstancer( pxr::UsdGeomPointInstancer &pointInstancer
 	prototypeRoots.reserve( targets.size() );
 	for( const auto &t : targets )
 	{
-		if( !gafferUSDPointInstancersRelativeProtoypes )
+		if( !g_relativePrototypes || !t.HasPrefix( primPath ) )
 		{
 			prototypeRoots.push_back( t.GetString() );
 		}
 		else
 		{
-			if( t.HasPrefix( primPath ) )
-			{
-				// The ./ prefix shouldn't be necessary - we want to just use the abscence of a leading
-				// slash to indicate relative paths. We can remove the prefix here once we deprecate the
-				// env var GAFFERSCENE_INSTANCER_EXPLICITABSOLUTEPATHS and have Gaffer always require a leading
-				// slash for absolute paths
-				prototypeRoots.push_back( "./" + t.MakeRelativePath( primPath ).GetString() );
-			}
-			else
-			{
-				prototypeRoots.push_back( t.GetString() );
-			}
+			// The ./ prefix shouldn't be necessary - we want to just use the absence of a leading
+			// slash to indicate relative paths. We can remove the prefix here once we deprecate the
+			// GAFFERSCENE_INSTANCER_EXPLICIT_ABSOLUTE_PATHS env var and have Gaffer always require a leading
+			// slash for absolute paths.
+			prototypeRoots.push_back( "./" + t.MakeRelativePath( primPath ).GetString() );
 		}
 	}
 

contrib/IECoreUSD/test/IECoreUSD/USDSceneTest.py

@@ -4488,38 +4488,39 @@ def testAssetPathSlashes ( self ) :
 		self.assertNotIn( "\\", xform.readAttribute( "render:testAsset", 0 ).value )
 		self.assertTrue( pathlib.Path( xform.readAttribute( "render:testAsset", 0 ).value ).is_file() )
 
+	def _testPointInstancerRelativePrototypes( self ) :
 
-	@unittest.skipIf( os.environ.get("IECOREUSD_POINTINSTANCER_RELATIVEPROTOTYPES", "0") == "0", "Set IECOREUSD_POINTINSTANCER_RELATIVEPROTOTYPES to run relative version of this test." )
-	def testPointInstancerRelative( self ) :
 		root = IECoreScene.SceneInterface.create(
 			os.path.join( os.path.dirname( __file__ ), "data", "pointInstancerWeirdPrototypes.usda" ),
 			IECore.IndexedIO.OpenMode.Read
 		)
 		pointInstancer = root.child( "inst" )
 		obj = pointInstancer.readObject(0.0)
-		self.assertEqual( obj["prototypeRoots"].data, IECore.StringVectorData( [ './Prototypes/sphere', '/cube' ] ) )
 
-	def testPointInstancerRelativeRunner( self ) :
-		env = os.environ.copy()
-		env["IECOREUSD_POINTINSTANCER_RELATIVEPROTOTYPES"] = "1"
+		if os.environ.get( "IECOREUSD_POINTINSTANCER_RELATIVE_PROTOTYPES", "0" ) != "0" :
+			self.assertEqual( obj["prototypeRoots"].data, IECore.StringVectorData( [ './Prototypes/sphere', '/cube' ] ) )
+		else :
+			self.assertEqual( obj["prototypeRoots"].data, IECore.StringVectorData( [ '/inst/Prototypes/sphere', '/cube' ] ) )
 
-		try :
-			subprocess.check_output(
-				[ sys.executable, __file__, "USDSceneTest.testPointInstancerRelative" ],
-				env = env, stderr = subprocess.STDOUT
-			)
-		except subprocess.CalledProcessError as e :
-			self.fail( e.output )
+	def testPointInstancerRelativePrototypes( self ) :
 
-	def testPointInstancerAbsolute( self ) :
-		root = IECoreScene.SceneInterface.create(
-			os.path.join( os.path.dirname( __file__ ), "data", "pointInstancerWeirdPrototypes.usda" ),
-			IECore.IndexedIO.OpenMode.Read
-		)
-		pointInstancer = root.child( "inst" )
-		obj = pointInstancer.readObject(0.0)
-		self.assertEqual( obj["prototypeRoots"].data, IECore.StringVectorData( [ '/inst/Prototypes/sphere', '/cube' ] ) )
+		for relative in [ "0", "1", None ] :
+
+			with self.subTest( relative = relative ) :
 
+				env = os.environ.copy()
+				if relative is not None :
+					env["IECOREUSD_POINTINSTANCER_RELATIVE_PROTOTYPES"] = relative
+				else :
+					env.pop( "IECOREUSD_POINTINSTANCER_RELATIVE_PROTOTYPES", None )
+
+				try :
+					subprocess.check_output(
+						[ sys.executable, __file__, "USDSceneTest._testPointInstancerRelativePrototypes" ],
+						env = env, stderr = subprocess.STDOUT
+					)
+				except subprocess.CalledProcessError as e :
+					self.fail( e.output )
 
 	@unittest.skipIf( not haveVDB, "No IECoreVDB" )
 	def testUsdVolVolumeSlashes( self ) :