Merge pull request #884 from eivindkv/Fix_issue_3128

Expressions: Fixed issue #3128; mod(x;y) should support arbitrary uni…
FreeCAD · Jul 18, 2017 · 382ad28 · 382ad28
2 parents 67060d6 + 72dd6df
commit 382ad28
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 4 deletions.
diff --git a/src/App/Expression.cpp b/src/App/Expression.cpp
@@ -1007,9 +1007,7 @@ Expression * FunctionExpression::eval() const
     case MOD:
         if (v2 == 0)
             throw ExpressionError("Invalid second argument.");
-        if (!v2->getUnit().isEmpty())
-            throw ExpressionError("Second argument must have empty unit.");
-        unit = v1->getUnit();
+        unit = v1->getUnit() / v2->getUnit();
         break;
     case POW: {
         if (v2 == 0)

diff --git a/src/Mod/Spreadsheet/TestSpreadsheet.py b/src/Mod/Spreadsheet/TestSpreadsheet.py
@@ -300,7 +300,7 @@ def testFunctions(self):
         self.assertMostlyEqual(sheet.A19, 3) # Mod
         self.assertMostlyEqual(sheet.B19, -3)
         self.assertMostlyEqual(sheet.C19, Units.Quantity('3 mm'))
-        self.assertEqual(sheet.D19, u'ERR: Second argument must have empty unit.')
+        self.assertEqual(sheet.D19, 3)
         self.assertMostlyEqual(sheet.A20, Units.Quantity('45 deg'))       # Atan2
         self.assertMostlyEqual(sheet.B20, Units.Quantity('-45 deg'))
         self.assertEqual(sheet.C20, u'ERR: Units must be equal')
@@ -775,6 +775,15 @@ def testCrossDocumentLinks(self):
         # Close second document
         FreeCAD.closeDocument(doc2.Name)
 
+    def testIssue3128(self):
+        """ Regression test for issue 3128; mod should work with arbitrary units for both arguments """
+        sheet = self.doc.addObject('Spreadsheet::Sheet','Spreadsheet')
+        sheet.set('A1', '=mod(7mm;3mm)')
+        sheet.set('A2', '=mod(7kg;3mm)')
+        self.doc.recompute()
+        self.assertEqual(sheet.A1, Units.Quantity('1'))
+        self.assertEqual(sheet.A2, Units.Quantity('1 kg/mm'))
+
     def tearDown(self):
         #closing doc
         FreeCAD.closeDocument(self.doc.Name)