FrameRange : Prevent creation of FrameRanges with negative steps

Previously, you could create a FrameRange object with a negative step,
but it would go into an infinite loop if you tried to get the frame list

Given that we have the ReversedFrameList that achieves the same as a
negative step, it felt like using that was the correct approach.

Note that we still include the support for the negative step in the
parse method, so that we can provide a more useful error message during
init.

Author: ivanimanishi

Changes

@@ -1,6 +1,11 @@
 10.4.x.x (relative to 10.4.10.0)
 ========
 
+Fixes
+-----
+
+- FrameRange : Prevented creation of FrameRanges with negative steps
+
 10.4.10.0 (relative to 10.4.9.1)
 ========
 

src/IECore/FrameRange.cpp

@@ -58,6 +58,10 @@ FrameRange::FrameRange( Frame start, Frame end, Frame step ) : m_start( start ),
 	{
 		throw Exception( "FrameRange step cannot be zero" );
 	}
+	if ( step < 0 )
+	{
+		throw Exception( "FrameRange step cannot be negative. Consider using the reverse suffix instead." );
+	}
 }
 
 FrameRange::~FrameRange()
@@ -102,6 +106,10 @@ void FrameRange::setStep( Frame step )
 	{
 		throw Exception( "FrameRange step cannot be zero" );
 	}
+	if ( step < 0 )
+	{
+		throw Exception( "FrameRange step cannot be negative. Consider using the reverse suffix instead." );
+	}
 	m_step = step;
 }
 

test/IECore/FrameList.py

@@ -55,7 +55,24 @@ def testReverseConstruction( self ) :
 		self.assertEqual( f.asList(), [ 5, 4, 3, 2, 1 ] )
 		self.assertEqual( IECore.frameListFromList( [ 5, 4, 3, 2, 1 ] ), f )
 
+	def testFrameRange( self ) :
+
+		f = IECore.FrameList.parse( "1-5" )
+		self.assertTrue( isinstance( f, IECore.FrameRange ) )
+		self.assertEqual( f.asList(), [ 1, 2, 3, 4, 5 ] )
+		# test step
+		f = IECore.FrameList.parse( "10-20x5" )
+		self.assertTrue( isinstance( f, IECore.FrameRange ) )
+		self.assertEqual( f.asList(), [ 10, 15, 20 ] )
+		# start must be smaller or equal to end
+		self.assertRaises( Exception, IECore.FrameList.parse, "5-1"  )
+		# step must be positive
+		self.assertRaises( Exception, IECore.FrameList.parse, "1-5x0" )
+		self.assertRaises( Exception, IECore.FrameList.parse, "1-5x-1" )
+		self.assertRaises( Exception, IECore.FrameList.parse, "5-1x-1" )
+
+
 	## \todo: there should probably be a lot more tests in here...
 
 if __name__ == "__main__":
-        unittest.main()
+	unittest.main()