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

RowReader.float is misleading #136

Open
witoldsz opened this issue May 4, 2023 · 0 comments
Open

RowReader.float is misleading #136

witoldsz opened this issue May 4, 2023 · 0 comments

Comments

@witoldsz
Copy link

witoldsz commented May 4, 2023

I do not know who invented this, but look:
C# – Characteristics of the floating-point types

C# type/keyword Approximate range Precision Size .NET type
float ±1.5 x 10−45 to ±3.4 x 1038 ~6-9 digits 4 bytes System.Single
double ±5.0 × 10−324 to ±1.7 × 10308 ~15-17 digits 8 bytes System.Double

and now look at what F# has:

F# – Basic types

Type .NET type Description Example
float, double Double A 64-bit floating point type. 1.0
float32, single Single A 32-bit floating point type. 1.0f

So:

  • C# float maps to F# float32 (or single) and
  • F# float maps to C# double. 🤯

This is crazy and can cause lots of troubles.

Now, I can see the RowReader follows this and row.float maps to F# float32 instead of float a.k.a double.

    /// Gets the value of the specified column as a `System.Single` object.
    member this.float(column: string) : float32 =
        this.fieldValue(column)

The problem is, the change could cause even more harm than good at this point, but I would recommend:

  • make RowReader's float deprecated (with explanation that the name is ambiguous)
  • use the single and maybe an alias float32 to describe 32-bit floating point conversion

With this change, we would avoid the contaminated float and also make the issue more expressive for those who are not aware of the trap. What would you say?

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

No branches or pull requests

1 participant