Skip to content

Commit c258091

Browse files
authored
align rem and quot methodologies with Clojure (#849)
Hi, could you please consider patch to align the `rem` and `quot` results with those of Clojure. It fixes #848. This supersedes the decision to have `quot` use floored division implemented #823. This is to opt for full compatibility with Clojure, rather than using numerically different but optimised python implementations. Users can decide to use the corresponding python operators if they would like to do otherwise. I've also added some new test cases to cover additional results. Thanks --------- Co-authored-by: ikappaki <[email protected]>
1 parent 08f7696 commit c258091

File tree

5 files changed

+75
-18
lines changed

5 files changed

+75
-18
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1212
### Changed
1313
* Cause exceptions arising from compilation issues during macroexpansion will no longer be nested for each level of macroexpansion (#852)
1414
* Support for optional metadata argument in `defmulti` (#857)
15+
* Aligned `rem` and `quot` methodologies with corresponding Clojure fns (#848)
1516

1617
### Fixed
1718
* Fix a bug where `basilisp.lang.compiler.exception.CompilerException` would nearly always suppress line information in it's `data` map (#845)

docs/differencesfromclojure.rst

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -133,13 +133,6 @@ Libs
133133

134134
Support for Clojure libs is `planned <https://github.com/basilisp-lang/basilisp/issues/668>`_\.
135135

136-
.. _core_lib_differences:
137-
138-
core
139-
----------
140-
141-
The :lpy:fn:`quot` function utilizes the ``\\`` operator, resulting in a floor operation towards negative infinity. In contrast, Clojure rounds the result towards zero. Consequently, negative results exhibit a difference of -1 between Basilisp and Clojure.
142-
143136
.. _refs_and_transactions_differences:
144137

145138
Refs and Transactions

docs/pyinterop.rst

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,4 +244,19 @@ Type hints may be applied to :lpy:form:`def` names, function arguments and retur
244244

245245
Due to the complexity of supporting multi-arity functions in Python, only return annotations are preserved on the arity dispatch function.
246246
Return annotations are combined as by ``typing.Union``, so ``typing.Union[str, str] == str``.
247-
The annotations for individual arity arguments are preserved in their compiled form, but they are challenging to access programmatically.
247+
The annotations for individual arity arguments are preserved in their compiled form, but they are challenging to access programmatically.
248+
249+
.. _arithmeticdivision
250+
251+
Arithmetic division
252+
-------------------
253+
254+
:lpy:fn:`basilisp.core/quot`, :lpy:fn:`basilisp.core/rem` and :lpy:fn:`basilisp.core/mod` functions.
255+
256+
The Python native quotient ``//`` and modulo ``%`` operators may yield different results compared to their Java counterpart's long division and modulo operators. The discrepancy arises from Python's choice of floored division (`src <http://python-history.blogspot.com/2010/08/why-pythons-integer-division-floors.html>`_, `archived <https://web.archive.org/web/20100827160949/http://python-history.blogspot.com/2010/08/why-pythons-integer-division-floors.html>`_) while Java employs truncated division for its calculations (refer to the to the `Wikipedia Modulo page <https://en.wikipedia.org/wiki/Modulo>`_ for a a comprehensive list of available division formulae).
257+
258+
In Clojure, the ``clojure.core/quot`` function utilizes Java's long division operator, and the ``%`` operator is employed in defining the ``clojure.core/rem`` function. The ``clojure.core/mod`` function is subsequently established through floored division based on the latter.
259+
260+
Basilisp has chosen to adopt the same mathematical formulae as Clojure for these three functions, rather than using the Python's built in operators under all cases. This approach offers the advantage of enhanced cross-platform compatibility without requiring modification, and ensures compatibility with examples in `ClojureDocs <https://clojuredocs.org/>`_.
261+
262+
Users still have the option to use the native `operator/floordiv <https://docs.python.org/3/library/operator.html#operator.floordiv>`_, i.e. Python's ``//`` operator, if they prefer so.

src/basilisp/core.lpy

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1089,21 +1089,25 @@
10891089
(python/abs x))
10901090

10911091
(defn ^:inline mod
1092-
"Returns the modulo of ``num`` and ``div``\\."
1092+
"Returns the modulo of ``num`` and ``div``\\.
1093+
1094+
It uses floored division for calculating the quotient."
10931095
[num div]
10941096
(operator/mod num div))
10951097

10961098
(defn ^:inline quot
10971099
"Returns the quotient of ``num`` and ``div``\\.
10981100

1099-
The result is rounded towards negative infinity."
1101+
The division result is truncated."
11001102
[num div]
1101-
(operator/floordiv num div))
1103+
(int (/ num div)))
1104+
1105+
(defn ^:inline rem
1106+
"Returns the remainder of ``num`` and ``div``\\.
11021107

1103-
(defn rem
1104-
"Returns the remainder of ``num`` and ``div``\\."
1108+
It uses truncated division for calculating the quotient."
11051109
[num div]
1106-
(math/remainder num div))
1110+
(- num (* div (int (/ num div)))))
11071111

11081112
(defn ^:inline inc
11091113
"Increment the argument by 1."

tests/basilisp/core_test.py

Lines changed: 48 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,18 @@ def test_division(self):
391391
(4, 10, 6),
392392
(0, 10, 10),
393393
(0, 10, -1),
394+
(1, 5, 2),
395+
(1, -5, 2),
396+
(-1, 5, -2),
397+
(-1, -5, -2),
398+
(2, 5, 3),
399+
(1, -5, 3),
400+
(-1, 5, -3),
401+
(-2, -5, -3),
402+
(1, 5, 4),
403+
(3, -5, 4),
404+
(-3, 5, -4),
405+
(-1, -5, -4),
394406
(3, -21, 4),
395407
(3, -2, 5),
396408
(2, -10, 3),
@@ -407,16 +419,28 @@ def test_mod(self, result, x, y):
407419
"result,x,y",
408420
[
409421
(0, 1, 2),
422+
(2, 5, 2),
423+
(-2, -5, 2),
424+
(-2, 5, -2),
425+
(2, -5, -2),
426+
(1, 5, 3),
427+
(-1, -5, 3),
428+
(-1, 5, -3),
429+
(1, -5, -3),
430+
(1, 5, 4),
431+
(-1, -5, 4),
432+
(-1, 5, -4),
433+
(1, -5, -4),
410434
(1, 2, 2),
411435
(1, 3, 2),
412436
(2, 4, 2),
413437
(3, 10, 3),
414438
(3, 11, 3),
415439
(4, 12, 3),
416440
(1.0, 5.9, 3),
417-
(-2.0, -5.9, 3),
418-
(-4, -10, 3),
419-
(-4, 10, -3),
441+
(-1.0, -5.9, 3),
442+
(-3, -10, 3),
443+
(-3, 10, -3),
420444
(3, 10, 3),
421445
(
422446
44879032948094820938438942938402938402984209842098984209449032094205874758758475837584759347,
@@ -429,7 +453,27 @@ def test_quot(self, result, x, y):
429453
assert result == core.quot(x, y)
430454

431455
@pytest.mark.parametrize(
432-
"result,x,y", [(1, 10, 9), (0, 2, 2), (-1, -10, 3), (-1, -21, 4)]
456+
"result,x,y",
457+
[
458+
(1, 10, 9),
459+
(0, 2, 2),
460+
(1, 5, 2),
461+
(-1, -5, 2),
462+
(1, 5, -2),
463+
(-1, -5, -2),
464+
(2, 5, 3),
465+
(-2, -5, 3),
466+
(2, 5, -3),
467+
(-2, -5, -3),
468+
(1, 5, 4),
469+
(-1, -5, 4),
470+
(1, 5, -4),
471+
(-1, -5, -4),
472+
(1, 3, 2),
473+
(-1, -3, 2),
474+
(-1, -10, 3),
475+
(-1, -21, 4),
476+
],
433477
)
434478
def test_rem(self, result, x, y):
435479
assert result == core.rem(x, y)

0 commit comments

Comments
 (0)