'numerator and 'denominator fail to handle integral values (i.e. N/1)

Question

Because ratio values reduce to lowest terms and, for integral values where the lowest term is N/1, are auto-converted to BigInts (and formerly Longs), the current behavior of {{clojure.core/numerator}} and {{clojure.core/denominator}} yield unexpected results.

`
user=> (numerator 1/3)
1
user=> (numerator (+ 1/3 2/3))

ClassCastException clojure.lang.BigInt cannot be cast to clojure.lang.Ratio clojure.core/numerator (core.clj:3306)
user=> (denominator 1/3)
3
user=> (denominator (+ 1/3 2/3))

ClassCastException clojure.lang.BigInt cannot be cast to clojure.lang.Ratio clojure.core/denominator (core.clj:3314)
user=>
`

It's confusing to not support numerator and denominator on integer types as this requires you to always check ratio? before invoking them.

Proposed: Extend numerator and denominator to also work on integer types (long, BigInt, BigInteger) by routing to overloaded methods on Numbers for the desired types.

Patch: clj-1435.patch

Screened by: Alex Miller

Questions from screening for Rich:

1. numerator and denominator are tagged as returning java.math.BigInteger (not clojure.lang.BigInt) and that's what I followed in the patch. Seems like maybe that should be BigInt though? Not sure on what basis to make that decision.
2. Should numerator and denominator accept both BigInteger and BigInt?

Answer 1

Comment made by: jafingerhut

I don't know the official stance on this ticket, but will add some notes.

Aaron, numerator and denominator are pretty clearly documented to work on Ratio types only.

It is pretty easy to write my-numerator and my-denominator that work exactly as you wish, checking for the type of arg and using numerator, denominator for Ratio types, and doing whatever you think is correct for other numeric types.

Answer 2

Comment made by: abrooks

I'm aware that they are documented as such. Part of my point is that you can be working entirely with Ratio types and, via arithmetic operations between them, sometimes wind up with a non-Ratio number unexpectedly.

Also consider:

user=> (numerator 2/1) ClassCastException java.lang.Long cannot be cast to clojure.lang.Ratio clojure.core/numerator (core.clj:3238)

You're then left either implementing a try/catch correction or always checking the type before using {{numerator}} or {{denominator}} which is a loss in performance.

The patch I have in mind is creating a protocol, extended to Ratio, BigInt and Long which calls the appropriate method (Ratios) or returns either the given number or 1 (numerator/denominator) for the integral types. I expect this to maintain the current level of performance in the cases where it works and behave properly in the cases currently not handled.

Answer 3

Comment made by: gfredericks

I've definitely written the helper functions Andy describes on several occasions.

Answer 4

Comment made by: felipelalli

Related issue: https://stackoverflow.com/questions/25194809/how-to-convert-any-number-to-a-clojure-lang-ratio-type-in-clojure

A workaround to that is (numerator (clojure.lang.Numbers/toRatio (rationalize )))

Answer 5

Comment made by: alexmiller

I agree with the intent of the ticket here that these should work. I'm not sure about the protocol approach as that would be an open system and I'm not sure that's what we want. An alternative would be to just create Java methods on Numbers that took the appropriate types and let the JVM sort it out.

Answer 6

Comment made by: abrooks

Any progress on this? :)

Answer 7

Comment made by: alexmiller

Sitting in the queue, waiting for love.

Answer 8

Comment made by: abrooks

Can I give it love? If there's a direction that we can agree on, I'd be happy to create the patch.

Answer 9

Comment made by: alexmiller

I've screeened the patch that's here already?

Answer 10

Comment made by: jafingerhut

The new versions of numerator and denominator after this patch was applied perform reflective calls. Is that expected?

Answer 11

Comment made by: jafingerhut

Created ticket CLJ-2408 to make the question more prominent than a comment on a closed ticket.

Answer 12

Comment made by: alexmiller

Patch reverted

Answer 13

Reference: https://clojure.atlassian.net/browse/CLJ-1435 (reported by abrooks)