{"action":"created","issue":{"url":"https://api.github.com/repos/JuliaAstro/FITSIO.jl/issues/25","labels_url":"https://api.github.com/repos/JuliaAstro/FITSIO.jl/issues/25/labels{/name}","comments_url":"https://api.github.com/repos/JuliaAstro/FITSIO.jl/issues/25/comments","events_url":"https://api.github.com/repos/JuliaAstro/FITSIO.jl/issues/25/events","html_url":"https://github.com/JuliaAstro/FITSIO.jl/pull/25","id":64874600,"number":25,"title":"WIP: non-standard header values","user":{"login":"kbarbary","id":970135,"avatar_url":"https://avatars.githubusercontent.com/u/970135?v=3","gravatar_id":"","url":"https://api.github.com/users/kbarbary","html_url":"https://github.com/kbarbary","followers_url":"https://api.github.com/users/kbarbary/followers","following_url":"https://api.github.com/users/kbarbary/following{/other_user}","gists_url":"https://api.github.com/users/kbarbary/gists{/gist_id}","starred_url":"https://api.github.com/users/kbarbary/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/kbarbary/subscriptions","organizations_url":"https://api.github.com/users/kbarbary/orgs","repos_url":"https://api.github.com/users/kbarbary/repos","events_url":"https://api.github.com/users/kbarbary/events{/privacy}","received_events_url":"https://api.github.com/users/kbarbary/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":1,"created_at":"2015-03-27T23:56:17Z","updated_at":"2015-03-30T06:26:19Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/JuliaAstro/FITSIO.jl/pulls/25","html_url":"https://github.com/JuliaAstro/FITSIO.jl/pull/25","diff_url":"https://github.com/JuliaAstro/FITSIO.jl/pull/25.diff","patch_url":"https://github.com/JuliaAstro/FITSIO.jl/pull/25.patch"},"body":"This is a simple fix for #24. To explain: cfitsio returns header values as strings. In the high-level interface `FITSHeader`, we parse these strings into their implied types, for convenience. First we try to parse as an int, then a float, then a bool, then nothing, then a string (if value is surrounded by `'`). What to do when we get to the end? Formerly we raised an error saying that the value couldn't be parsed. This PR changes the behavior to simply return the unparsed string.\r\n\r\nThe upside of the change is that a single malformed keyword will no longer prevent one from reading the entire header. The slight downside is that you can't tell if the original unparsed value was a correctly quoted string or a malformed value that we gave up parsing. The upside outweighs the downside, but it made me think about whether we should keep the unparsed header in memory so that we can write it back out *exactly* as the original.\r\n\r\n~~While checking the behavior of this non-standard file I also ran into another bug that I'll add a fix for here before we merge.~~ Done (see the commit about writing missing values)."},"comment":{"url":"https://api.github.com/repos/JuliaAstro/FITSIO.jl/issues/comments/87567397","html_url":"https://github.com/JuliaAstro/FITSIO.jl/pull/25#issuecomment-87567397","issue_url":"https://api.github.com/repos/JuliaAstro/FITSIO.jl/issues/25","id":87567397,"user":{"login":"kbarbary","id":970135,"avatar_url":"https://avatars.githubusercontent.com/u/970135?v=3","gravatar_id":"","url":"https://api.github.com/users/kbarbary","html_url":"https://github.com/kbarbary","followers_url":"https://api.github.com/users/kbarbary/followers","following_url":"https://api.github.com/users/kbarbary/following{/other_user}","gists_url":"https://api.github.com/users/kbarbary/gists{/gist_id}","starred_url":"https://api.github.com/users/kbarbary/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/kbarbary/subscriptions","organizations_url":"https://api.github.com/users/kbarbary/orgs","repos_url":"https://api.github.com/users/kbarbary/repos","events_url":"https://api.github.com/users/kbarbary/events{/privacy}","received_events_url":"https://api.github.com/users/kbarbary/received_events","type":"User","site_admin":false},"created_at":"2015-03-30T06:26:19Z","updated_at":"2015-03-30T06:26:19Z","body":"I disagree that we should emit a warning. Warnings can be useful when they're something that a user can fix (deprecation warnings, for example). In this case, the user almost certainly has no control over the format of the file they're reading, so she or he can't do anything to make the warning go away. From experience with some FITS parsers, this can be quite annoying. Furthermore, there might only be one keyword record that was non-standard; it's quite possible (probable, even) that the user isn't concerned with the value of that specific record, and that the rest of the header parsed just fine.\r\n\r\nIf it helps, I don't think of the new behavior as \"ignoring an error.\" Rather, we are redefining the parsing behavior as follows:\r\n\r\n- starting from the value field in string form, attempt to parse it according to the FITS standard\r\n- if the value is non-standards compliant, return the unparsed value string\r\n\r\nI think simply clearly documenting this behavior should be enough (and I can add this to the docs)."}}