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

Description

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.

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?

Environment

None

Activity

Show:
Aaron Brooks
April 8, 2017, 9:54 PM

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

Alex Miller
April 9, 2017, 12:13 AM

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

Andy Fingerhut
September 27, 2018, 10:32 AM

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

Andy Fingerhut
September 27, 2018, 11:06 AM

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

Alex Miller
October 6, 2018, 4:58 AM

Patch reverted

Assignee

Unassigned

Reporter

Aaron Brooks

Labels

Approval

Vetted

Patch

Code and Test

Affects versions

Priority

Major
Configure