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

Floats better last? #1

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

Conversation

PallHaraldsson
Copy link

No description provided.

@PallHaraldsson
Copy link
Author

It's a rather trivial change, but I wasn't even sure why you would need so much to change Float64. But if you do, would you want to change Float32, and possibly Float16 too? I thought it might be awkward to have int in-between. Since the code is brand-new, and people haven't used much yet (and not registered) might be a good time to change, if you ever do.

Could add keyword arguments too.

@rfourquet
Copy link
Owner

Thanks for your suggestion!

Actually, I'm not even sure I want to keep this positional-arguments method, the pairs version is clearer. The reasoning for how it is now is that I assumed BigInt would be changed less often than Float64, so Float64 should come first among the two. And having all integers one after the other is better than having Float64 in between (as you noted), so Float64 came first. I prefer getting more user feed-back before starting moving things around; and indeed we might decide to just delete the positional-only method, or to have it be (Int, Int128, Float64) (without BigInt).

In any case, I don't want to overload this method (which should remain simple) with Float32 and Float16 (which don't even have literals!). The point of this method is to handle the "defaults", i.e. these literals which are not annotated (e.g. for Float32 you have to explicitly add the f0 suffix).

Could add keyword arguments too.

Actually this was first implemented with keyword arguments, and this is still available but not documented. But @cmcaine suggested that using pairs could be clearer, and I tend to agree. I didn't delete yet the kwargs version as it's still more terse, and more experience with the package might reveal that kwargs has its place, but I think the pairs approach is superior.

@PallHaraldsson
Copy link
Author

After our discussion on Discourse, I would (at least for now) simply change floats to be unchanged as a default. It could be related to the PR change here, or not.

It's still "safe", just surprising to beginners. At least BigFloats doesn't makes safer, just avoids overflow (not a big problem with floats), and has problems.

@rfourquet rfourquet force-pushed the master branch 4 times, most recently from ff4cd06 to f8d8c22 Compare November 7, 2020 08:57
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