Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update functions.jl #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

DrDrDrDrDrAlhazenEuler
Copy link

The "show" method did not function properly in Julia 1.10.5; the if clause should have been unnecessary anyhow, because Julia allows overloading and x is explicitly specified to be a BigRational.

The "show" method did not function properly in Julia 1.10.5; the if clause should have been unnecessary anyhow, because Julia allows overloading and x is explicitly specified to be a BigRational.
@Liozou
Copy link
Owner

Liozou commented Dec 3, 2024

Thanks for the contribution and sorry for the delay! Would you mind explaining what isn't properly working with the current show method?

For context, as the documentation of show specifies:

The representation used by show generally includes Julia-specific formatting and type information, and should be parseable Julia code when possible.

This is the reason why BigRational(12, 5) for example is represented by BigRational(12,5) and not 12//5, otherwise the representation of BigRational(12, 5) would lose the type information.
However, when showing an array, the type information can be put externally, which is why, with the current representation, show([BigRational(12, 5), BigRational(7)]) displays BigRational[12//5, 7//1].
This is the same reason why show(Bool(1)) displays true and show([Bool(1), Bool(0)]) displays Bool[1, 0].

By the way, most of the contents of this package has been moved upstream: you can now access GMP's rational functions from the Base.GMP.MPQ module, operating on Rational{BigInt}. I'm happy of course if this package can still be useful, but if you only need the most basic operations, you should probably simply use the Rational{BigInt} type instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants