Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

Serialising NaN float produces invalid JSON. #988

Closed
SimonSchneider opened this issue May 11, 2022 · 2 comments · Fixed by #990
Closed

Serialising NaN float produces invalid JSON. #988

SimonSchneider opened this issue May 11, 2022 · 2 comments · Fixed by #990
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed no-changelog Issues whose changes are covered by a PR and thus should not be shown in the changelog

Comments

@SimonSchneider
Copy link
Contributor

SimonSchneider commented May 11, 2022

Serialising an arrow vector (?) of dataType Float with a NaN value produces invalid JSON.

I hope it's ok that I have a test from Polars as that is the way I interact with arrow.

    #[test]
    fn serialize_row_with_nan_float() -> Result<()> {
        let mut df = df! {
            "a" => [4.0, 0.0,f64::NAN]
        }?;
        let mut buf = Vec::new();
        JsonWriter::new(&mut buf)
            .with_json_format(JsonFormat::Json)
            .finish(&mut df)?;
        let encoded = std::str::from_utf8(&buf).unwrap();
        dbg!(encoded);
        let test_res = serde_json::json!([4.0, 0.0, f64::NAN]);
        dbg!(test_res);
        let res: serde_json::Value = serde_json::from_slice(&buf)?;
        dbg!(res);
        Ok(())
    }

produces:

[src/test.rs:775] encoded = "[{\"a\":4.0},{\"a\":0.0},{\"a\":NaN}]"
[src/test.rs:778] test_res = Array([
    Number(
        4.0,
    ),
    Number(
        0.0,
    ),
    Null,
])
Error: expected value at line 1 column 27

Above can be seen that the arrow JSON wrote: {\"a\":NaN} but the normal rust serde_json writes Null for the NaN value.

If needed I can try rewriting the test into arrow2 specific code.

@jorgecarleitao jorgecarleitao added bug Something isn't working help wanted Extra attention is needed good first issue Good for newcomers labels May 12, 2022
@jorgecarleitao
Copy link
Owner

Indeed - well spotted. This is a good first issue - let me know if you would like to PR :)

@SimonSchneider
Copy link
Contributor Author

Thanks, I’ll gladly take a look over the weekend, but I might be a little out of my depth so I might have some questions. I’ll get back to this thread or open a PR.

@jorgecarleitao jorgecarleitao added the no-changelog Issues whose changes are covered by a PR and thus should not be shown in the changelog label May 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed no-changelog Issues whose changes are covered by a PR and thus should not be shown in the changelog
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants