{"action":"created","issue":{"url":"https://api.github.com/repos/astropy/astropy/issues/7251","repository_url":"https://api.github.com/repos/astropy/astropy","labels_url":"https://api.github.com/repos/astropy/astropy/issues/7251/labels{/name}","comments_url":"https://api.github.com/repos/astropy/astropy/issues/7251/comments","events_url":"https://api.github.com/repos/astropy/astropy/issues/7251/events","html_url":"https://github.com/astropy/astropy/pull/7251","id":301562251,"number":7251,"title":"WIP: Adding TDISPn translation to Python format strings","user":{"login":"SaraOgaz","id":5287848,"avatar_url":"https://avatars1.githubusercontent.com/u/5287848?v=4","gravatar_id":"","url":"https://api.github.com/users/SaraOgaz","html_url":"https://github.com/SaraOgaz","followers_url":"https://api.github.com/users/SaraOgaz/followers","following_url":"https://api.github.com/users/SaraOgaz/following{/other_user}","gists_url":"https://api.github.com/users/SaraOgaz/gists{/gist_id}","starred_url":"https://api.github.com/users/SaraOgaz/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/SaraOgaz/subscriptions","organizations_url":"https://api.github.com/users/SaraOgaz/orgs","repos_url":"https://api.github.com/users/SaraOgaz/repos","events_url":"https://api.github.com/users/SaraOgaz/events{/privacy}","received_events_url":"https://api.github.com/users/SaraOgaz/received_events","type":"User","site_admin":false},"labels":[{"id":373761720,"url":"https://api.github.com/repos/astropy/astropy/labels/Work%20in%20progress","name":"Work in progress","color":"f9d0c4","default":false},{"id":3496818,"url":"https://api.github.com/repos/astropy/astropy/labels/io.fits","name":"io.fits","color":"c7def8","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":6,"created_at":"2018-03-01T21:16:05Z","updated_at":"2018-03-07T16:19:21Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/astropy/astropy/pulls/7251","html_url":"https://github.com/astropy/astropy/pull/7251","diff_url":"https://github.com/astropy/astropy/pull/7251.diff","patch_url":"https://github.com/astropy/astropy/pull/7251.patch"},"body":"see history in #7230\r\nSo, here's the first two functions that will be needed for this.  \"This\" being parsing the TDSIPn keywords.  But I'm not totally sure how to preceed from here.\r\n\r\nThe background for those not familiar is.... I need the TDISPn keywords to roundtrip from FITS binary table to Astropy Table.  Right now, they don't get included in the Astropy Table at all.  The hope was to move the tdisp value to the `format` attribute of the Astropy Table.  When searching for any other code that already did this translation I landed on the code in column.py for `fits.io`, which did something sort of similar for TFORM (but with data types).  So it was suggested that I add this functionality into the column.py file, since it was on the to-do list anyway to parse the tdisp keyword.\r\n\r\nNow for the complication part....\r\nThere are several formats in the FORTRAN format standard that will not (as far as I can tell) round trip to a pre-set Python format string without losing some information.  For example, in the FORTRAN format there is a way to define an exponential format where you specify the exponent value explicitely, along with the zero padding AND space padding. \r\n\r\nSo, if I just over-write the `tdsip` attribute currently in the FITS IO `Column` class with the Pythonized format string, we can't get  back the original string without storing some extra information. I had a long conversation about this with @eteq, and he suggested we could write a view function that would edit the display format only if the user requested it.  And we could leave the `tdisp` attribute completely un-touched so that the roundtripping of the keyword in the header proceeds as normal. I'm not totally sure how to implement this since it seems like it would require a conditional call to overwrite `__repr__` (is that even possible?), but maybe I\"m mixing up some of the things @eteq and I talked about (this is probably what's happening).\r\n\r\nI don't need this functionality to solve my original problem for Astropy `Table`s, (#6806, #7226) and could definitely just use these functions inside the `Table` class to do what I need to do.  But if someone has a path foward for putting it here, and it won't take any major refactoring of the classes in this file, I'm up for working on it more.\r\n\r\n(FYI, I know I still need to add more comments and doc strings to these functions, as well as write some tests, was holding off on that until I knew what was going to happen to them).\r\n\r\ncc @saimn, @mhvk "},"comment":{"url":"https://api.github.com/repos/astropy/astropy/issues/comments/371192983","html_url":"https://github.com/astropy/astropy/pull/7251#issuecomment-371192983","issue_url":"https://api.github.com/repos/astropy/astropy/issues/7251","id":371192983,"user":{"login":"SaraOgaz","id":5287848,"avatar_url":"https://avatars1.githubusercontent.com/u/5287848?v=4","gravatar_id":"","url":"https://api.github.com/users/SaraOgaz","html_url":"https://github.com/SaraOgaz","followers_url":"https://api.github.com/users/SaraOgaz/followers","following_url":"https://api.github.com/users/SaraOgaz/following{/other_user}","gists_url":"https://api.github.com/users/SaraOgaz/gists{/gist_id}","starred_url":"https://api.github.com/users/SaraOgaz/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/SaraOgaz/subscriptions","organizations_url":"https://api.github.com/users/SaraOgaz/orgs","repos_url":"https://api.github.com/users/SaraOgaz/repos","events_url":"https://api.github.com/users/SaraOgaz/events{/privacy}","received_events_url":"https://api.github.com/users/SaraOgaz/received_events","type":"User","site_admin":false},"created_at":"2018-03-07T16:19:21Z","updated_at":"2018-03-07T16:19:21Z","author_association":"CONTRIBUTOR","body":"My plan was to have the conversion in the connect.py file (which I guess is technically still in io.fits not in the Tables module, heh) when reading a FITS to an Astropy `Table`, and wherever I would need the opposite conversion (I think that's in the same file).  It's true they won't convert one-to-one, but I feel like it's more acceptable to have changes like that to a keyword going to and from an Astropy `Table` then it happening in FITS IO directly.\r\n\r\nThe other alternative would be to make sure the original TDSIP gets stored in the `Table` meta-data, and re-written out from there when writing back to FITS, but also initially fill in the `format` `Column` attribute using TDISP.  There's some other issues with this though. Do we care that if someone updated `format` that it didn't update the stored TDISP, and make it clear to users that if they want to update TDISP they have to do it explicitly?  The whole thing gets a little dicey.\r\n\r\n@saimn thoughts?"}}
{"actor":{"display_login":"SaraOgaz"}}