{"action":"created","comment":{"url":"https://api.github.com/repos/astropy/astropy/pulls/comments/153719312","pull_request_review_id":79766226,"id":153719312,"diff_hunk":"@@ -217,6 +218,9 @@ def __array_finalize__(self, obj):\n         if obj is None:\n             return\n \n+        if isinstance(obj, FITS_rec):\n+            self._character_as_bytes = obj._character_as_bytes\n+\n         if isinstance(obj, FITS_rec) and obj.dtype == self.dtype:","path":"astropy/io/fits/fitsrec.py","position":15,"original_position":15,"commit_id":"9ac94811efb4c36c14c44c2fabec21102573a8fd","original_commit_id":"9ac94811efb4c36c14c44c2fabec21102573a8fd","user":{"login":"astrofrog","id":314716,"avatar_url":"https://avatars2.githubusercontent.com/u/314716?v=4","gravatar_id":"","url":"https://api.github.com/users/astrofrog","html_url":"https://github.com/astrofrog","followers_url":"https://api.github.com/users/astrofrog/followers","following_url":"https://api.github.com/users/astrofrog/following{/other_user}","gists_url":"https://api.github.com/users/astrofrog/gists{/gist_id}","starred_url":"https://api.github.com/users/astrofrog/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/astrofrog/subscriptions","organizations_url":"https://api.github.com/users/astrofrog/orgs","repos_url":"https://api.github.com/users/astrofrog/repos","events_url":"https://api.github.com/users/astrofrog/events{/privacy}","received_events_url":"https://api.github.com/users/astrofrog/received_events","type":"User","site_admin":false},"body":"Ah but that changes the logic, because the ``else`` needs to get executed if ``isinstance(obj, FITS_rec) and not obj.dtype == self.dtype``","created_at":"2017-11-29T08:32:40Z","updated_at":"2017-11-29T08:32:40Z","html_url":"https://github.com/astropy/astropy/pull/6821#discussion_r153719312","pull_request_url":"https://api.github.com/repos/astropy/astropy/pulls/6821","author_association":"OWNER","_links":{"self":{"href":"https://api.github.com/repos/astropy/astropy/pulls/comments/153719312"},"html":{"href":"https://github.com/astropy/astropy/pull/6821#discussion_r153719312"},"pull_request":{"href":"https://api.github.com/repos/astropy/astropy/pulls/6821"}},"in_reply_to_id":153579681},"pull_request":{"url":"https://api.github.com/repos/astropy/astropy/pulls/6821","id":151472281,"html_url":"https://github.com/astropy/astropy/pull/6821","diff_url":"https://github.com/astropy/astropy/pull/6821.diff","patch_url":"https://github.com/astropy/astropy/pull/6821.patch","issue_url":"https://api.github.com/repos/astropy/astropy/issues/6821","number":6821,"state":"open","locked":false,"title":"Make it possible to use Table.read on FITS files with no copying","user":{"login":"astrofrog","id":314716,"avatar_url":"https://avatars2.githubusercontent.com/u/314716?v=4","gravatar_id":"","url":"https://api.github.com/users/astrofrog","html_url":"https://github.com/astrofrog","followers_url":"https://api.github.com/users/astrofrog/followers","following_url":"https://api.github.com/users/astrofrog/following{/other_user}","gists_url":"https://api.github.com/users/astrofrog/gists{/gist_id}","starred_url":"https://api.github.com/users/astrofrog/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/astrofrog/subscriptions","organizations_url":"https://api.github.com/users/astrofrog/orgs","repos_url":"https://api.github.com/users/astrofrog/repos","events_url":"https://api.github.com/users/astrofrog/events{/privacy}","received_events_url":"https://api.github.com/users/astrofrog/received_events","type":"User","site_admin":false},"body":"### Motivation\r\n\r\nThe intent of this PR was to address (at least from the FITS point of view) the issue described in https://github.com/astropy/astropy/issues/6491 - namely that when using ``Table.read`` with a FITS file, the data is currently copied, which can lead to poor performance.\r\n\r\n### Bottom line\r\n\r\nWith this PR, I can read a 1.8Gb FITS table with some string columns using:\r\n\r\n```python\r\nfrom astropy.table import Table\r\nt = Table.read('../Planes/fun-with-adsb/summary_full.fits')\r\n```\r\n\r\nin 2 seconds with 40Mb memory usage:\r\n\r\n![after](https://user-images.githubusercontent.com/314716/32599495-a6059680-c534-11e7-91b6-ee225d71dcf1.png)\r\n\r\ninstead of 50+ seconds and 5Gb of memory:\r\n\r\n![before](https://user-images.githubusercontent.com/314716/32567694-f3f6a16c-c4b3-11e7-960b-cc201fb2baaf.png)\r\n\r\nIn fact, the 2 seconds/40Mb are dominated almost entirely by the import statement, and the read statement takes 40ms. This is a speedup of almost ~1000x! Of course data will then be read as needed from disk, but this means that for accessing specific columns/rows/cells from a table, users could see significant speedup.\r\n\r\n### FITS changes\r\n\r\nThis turned out to be more complicated than I thought. The issue is the following - we can simply add ``copy=False`` inside ``astropy/io/fits/connect.py`` to deal with preventing a copy for non-string columns. However, astropy.io.fits is inefficient when it comes to string columns in that it automatically converts ``A`` columns (which are fundamentally ASCII as far as I understand) to Numpy U(nicode) types. This was described by @taldcroft in: https://github.com/astropy/astropy/issues/6122 - this has two implications: memory mapping is not respected and the whole string columns are read in, and the memory usage is 4x what it needs to be because of the conversion to unicode.\r\n\r\nSo this PR adds the ability to control whether string columns are converted to unicode in astropy.io.fits with the ``return_bytes`` option to ``fits.open`` (which should be useful in itself), and then makes it so that we use that option for Table.read.\r\n\r\n### Table changes\r\n\r\nI also had to make it possible for ``add_column`` to take a ``copy`` argument - @taldcroft: was there a reason for not having that originally?\r\n\r\n### Time changes\r\n\r\nFinally I noticed that there was an issue in the Time parsing from FITS files - passing a Numpy array with S type to Time doesn't actually work so I needed to decode the values. This is inefficient but fixes what I think was a real bug.\r\n\r\n### Issues\r\n\r\nA few tests are now failing, which are I think related to comparisons with the unicode sandwich comparisons. @taldcroft, could you take a look at the failures since you are the most familiar with that code?\r\n\r\n### Testing\r\n\r\nA number of changes here require their own unit tests, which I can add if we decide to go ahead with this PR, along with changelog entries.\r\n\r\n### Review\r\n\r\nThis needs a careful look by the FITS maintainers: @saimn @MSeifert04 @drdavella, as well as Table maintainers @taldcroft and @mhvk, and @AustereCuriosity because of the FITS/Time change.\r\n\r\nAlso @adrn - please test this out (since your comment on Slack prompted me to work on this!)\r\n\r\n(this description was edited compared to its original version)\r\n\r\nCloses #6122","created_at":"2017-11-08T19:23:51Z","updated_at":"2017-11-29T08:32:40Z","closed_at":null,"merged_at":null,"merge_commit_sha":"3e88fe8d1fbf7d7c92eb9e7b4cb8cf3226f00dd1","assignee":null,"assignees":[],"requested_reviewers":[{"login":"adrn","id":583379,"avatar_url":"https://avatars0.githubusercontent.com/u/583379?v=4","gravatar_id":"","url":"https://api.github.com/users/adrn","html_url":"https://github.com/adrn","followers_url":"https://api.github.com/users/adrn/followers","following_url":"https://api.github.com/users/adrn/following{/other_user}","gists_url":"https://api.github.com/users/adrn/gists{/gist_id}","starred_url":"https://api.github.com/users/adrn/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/adrn/subscriptions","organizations_url":"https://api.github.com/users/adrn/orgs","repos_url":"https://api.github.com/users/adrn/repos","events_url":"https://api.github.com/users/adrn/events{/privacy}","received_events_url":"https://api.github.com/users/adrn/received_events","type":"User","site_admin":false},{"login":"drdavella","id":2458487,"avatar_url":"https://avatars0.githubusercontent.com/u/2458487?v=4","gravatar_id":"","url":"https://api.github.com/users/drdavella","html_url":"https://github.com/drdavella","followers_url":"https://api.github.com/users/drdavella/followers","following_url":"https://api.github.com/users/drdavella/following{/other_user}","gists_url":"https://api.github.com/users/drdavella/gists{/gist_id}","starred_url":"https://api.github.com/users/drdavella/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/drdavella/subscriptions","organizations_url":"https://api.github.com/users/drdavella/orgs","repos_url":"https://api.github.com/users/drdavella/repos","events_url":"https://api.github.com/users/drdavella/events{/privacy}","received_events_url":"https://api.github.com/users/drdavella/received_events","type":"User","site_admin":false},{"login":"MSeifert04","id":14200878,"avatar_url":"https://avatars2.githubusercontent.com/u/14200878?v=4","gravatar_id":"","url":"https://api.github.com/users/MSeifert04","html_url":"https://github.com/MSeifert04","followers_url":"https://api.github.com/users/MSeifert04/followers","following_url":"https://api.github.com/users/MSeifert04/following{/other_user}","gists_url":"https://api.github.com/users/MSeifert04/gists{/gist_id}","starred_url":"https://api.github.com/users/MSeifert04/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/MSeifert04/subscriptions","organizations_url":"https://api.github.com/users/MSeifert04/orgs","repos_url":"https://api.github.com/users/MSeifert04/repos","events_url":"https://api.github.com/users/MSeifert04/events{/privacy}","received_events_url":"https://api.github.com/users/MSeifert04/received_events","type":"User","site_admin":false}],"milestone":{"url":"https://api.github.com/repos/astropy/astropy/milestones/45","html_url":"https://github.com/astropy/astropy/milestone/45","labels_url":"https://api.github.com/repos/astropy/astropy/milestones/45/labels","id":2300822,"number":45,"title":"v3.0.0","description":null,"creator":{"login":"astrofrog","id":314716,"avatar_url":"https://avatars2.githubusercontent.com/u/314716?v=4","gravatar_id":"","url":"https://api.github.com/users/astrofrog","html_url":"https://github.com/astrofrog","followers_url":"https://api.github.com/users/astrofrog/followers","following_url":"https://api.github.com/users/astrofrog/following{/other_user}","gists_url":"https://api.github.com/users/astrofrog/gists{/gist_id}","starred_url":"https://api.github.com/users/astrofrog/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/astrofrog/subscriptions","organizations_url":"https://api.github.com/users/astrofrog/orgs","repos_url":"https://api.github.com/users/astrofrog/repos","events_url":"https://api.github.com/users/astrofrog/events{/privacy}","received_events_url":"https://api.github.com/users/astrofrog/received_events","type":"User","site_admin":false},"open_issues":63,"closed_issues":147,"state":"open","created_at":"2017-02-05T19:05:19Z","updated_at":"2017-11-28T22:53:43Z","due_on":null,"closed_at":null},"commits_url":"https://api.github.com/repos/astropy/astropy/pulls/6821/commits","review_comments_url":"https://api.github.com/repos/astropy/astropy/pulls/6821/comments","review_comment_url":"https://api.github.com/repos/astropy/astropy/pulls/comments{/number}","comments_url":"https://api.github.com/repos/astropy/astropy/issues/6821/comments","statuses_url":"https://api.github.com/repos/astropy/astropy/statuses/9ac94811efb4c36c14c44c2fabec21102573a8fd","head":{"label":"astrofrog:table-fits-nocopy","ref":"table-fits-nocopy","sha":"9ac94811efb4c36c14c44c2fabec21102573a8fd","user":{"login":"astrofrog","id":314716,"avatar_url":"https://avatars2.githubusercontent.com/u/314716?v=4","gravatar_id":"","url":"https://api.github.com/users/astrofrog","html_url":"https://github.com/astrofrog","followers_url":"https://api.github.com/users/astrofrog/followers","following_url":"https://api.github.com/users/astrofrog/following{/other_user}","gists_url":"https://api.github.com/users/astrofrog/gists{/gist_id}","starred_url":"https://api.github.com/users/astrofrog/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/astrofrog/subscriptions","organizations_url":"https://api.github.com/users/astrofrog/orgs","repos_url":"https://api.github.com/users/astrofrog/repos","events_url":"https://api.github.com/users/astrofrog/events{/privacy}","received_events_url":"https://api.github.com/users/astrofrog/received_events","type":"User","site_admin":false},"repo":{"id":2576841,"name":"astropy","full_name":"astrofrog/astropy","owner":{"login":"astrofrog","id":314716,"avatar_url":"https://avatars2.githubusercontent.com/u/314716?v=4","gravatar_id":"","url":"https://api.github.com/users/astrofrog","html_url":"https://github.com/astrofrog","followers_url":"https://api.github.com/users/astrofrog/followers","following_url":"https://api.github.com/users/astrofrog/following{/other_user}","gists_url":"https://api.github.com/users/astrofrog/gists{/gist_id}","starred_url":"https://api.github.com/users/astrofrog/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/astrofrog/subscriptions","organizations_url":"https://api.github.com/users/astrofrog/orgs","repos_url":"https://api.github.com/users/astrofrog/repos","events_url":"https://api.github.com/users/astrofrog/events{/privacy}","received_events_url":"https://api.github.com/users/astrofrog/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/astrofrog/astropy","description":"Main AstroPy repository","fork":true,"url":"https://api.github.com/repos/astrofrog/astropy","forks_url":"https://api.github.com/repos/astrofrog/astropy/forks","keys_url":"https://api.github.com/repos/astrofrog/astropy/keys{/key_id}","collaborators_url":"https://api.github.com/repos/astrofrog/astropy/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/astrofrog/astropy/teams","hooks_url":"https://api.github.com/repos/astrofrog/astropy/hooks","issue_events_url":"https://api.github.com/repos/astrofrog/astropy/issues/events{/number}","events_url":"https://api.github.com/repos/astrofrog/astropy/events","assignees_url":"https://api.github.com/repos/astrofrog/astropy/assignees{/user}","branches_url":"https://api.github.com/repos/astrofrog/astropy/branches{/branch}","tags_url":"https://api.github.com/repos/astrofrog/astropy/tags","blobs_url":"https://api.github.com/repos/astrofrog/astropy/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/astrofrog/astropy/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/astrofrog/astropy/git/refs{/sha}","trees_url":"https://api.github.com/repos/astrofrog/astropy/git/trees{/sha}","statuses_url":"https://api.github.com/repos/astrofrog/astropy/statuses/{sha}","languages_url":"https://api.github.com/repos/astrofrog/astropy/languages","stargazers_url":"https://api.github.com/repos/astrofrog/astropy/stargazers","contributors_url":"https://api.github.com/repos/astrofrog/astropy/contributors","subscribers_url":"https://api.github.com/repos/astrofrog/astropy/subscribers","subscription_url":"https://api.github.com/repos/astrofrog/astropy/subscription","commits_url":"https://api.github.com/repos/astrofrog/astropy/commits{/sha}","git_commits_url":"https://api.github.com/repos/astrofrog/astropy/git/commits{/sha}","comments_url":"https://api.github.com/repos/astrofrog/astropy/comments{/number}","issue_comment_url":"https://api.github.com/repos/astrofrog/astropy/issues/comments{/number}","contents_url":"https://api.github.com/repos/astrofrog/astropy/contents/{+path}","compare_url":"https://api.github.com/repos/astrofrog/astropy/compare/{base}...{head}","merges_url":"https://api.github.com/repos/astrofrog/astropy/merges","archive_url":"https://api.github.com/repos/astrofrog/astropy/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/astrofrog/astropy/downloads","issues_url":"https://api.github.com/repos/astrofrog/astropy/issues{/number}","pulls_url":"https://api.github.com/repos/astrofrog/astropy/pulls{/number}","milestones_url":"https://api.github.com/repos/astrofrog/astropy/milestones{/number}","notifications_url":"https://api.github.com/repos/astrofrog/astropy/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/astrofrog/astropy/labels{/name}","releases_url":"https://api.github.com/repos/astrofrog/astropy/releases{/id}","deployments_url":"https://api.github.com/repos/astrofrog/astropy/deployments","created_at":"2011-10-14T14:58:26Z","updated_at":"2016-12-20T14:00:17Z","pushed_at":"2017-11-27T23:56:35Z","git_url":"git://github.com/astrofrog/astropy.git","ssh_url":"git@github.com:astrofrog/astropy.git","clone_url":"https://github.com/astrofrog/astropy.git","svn_url":"https://github.com/astrofrog/astropy","homepage":"astropy.org","size":57156,"stargazers_count":4,"watchers_count":4,"language":"Python","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"archived":false,"open_issues_count":0,"license":null,"forks":0,"open_issues":0,"watchers":4,"default_branch":"master"}},"base":{"label":"astropy:master","ref":"master","sha":"63fc64d92c1f7180265c3372e786d6b531c1aad1","user":{"login":"astropy","id":847984,"avatar_url":"https://avatars0.githubusercontent.com/u/847984?v=4","gravatar_id":"","url":"https://api.github.com/users/astropy","html_url":"https://github.com/astropy","followers_url":"https://api.github.com/users/astropy/followers","following_url":"https://api.github.com/users/astropy/following{/other_user}","gists_url":"https://api.github.com/users/astropy/gists{/gist_id}","starred_url":"https://api.github.com/users/astropy/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/astropy/subscriptions","organizations_url":"https://api.github.com/users/astropy/orgs","repos_url":"https://api.github.com/users/astropy/repos","events_url":"https://api.github.com/users/astropy/events{/privacy}","received_events_url":"https://api.github.com/users/astropy/received_events","type":"Organization","site_admin":false},"repo":{"id":2081289,"name":"astropy","full_name":"astropy/astropy","owner":{"login":"astropy","id":847984,"avatar_url":"https://avatars0.githubusercontent.com/u/847984?v=4","gravatar_id":"","url":"https://api.github.com/users/astropy","html_url":"https://github.com/astropy","followers_url":"https://api.github.com/users/astropy/followers","following_url":"https://api.github.com/users/astropy/following{/other_user}","gists_url":"https://api.github.com/users/astropy/gists{/gist_id}","starred_url":"https://api.github.com/users/astropy/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/astropy/subscriptions","organizations_url":"https://api.github.com/users/astropy/orgs","repos_url":"https://api.github.com/users/astropy/repos","events_url":"https://api.github.com/users/astropy/events{/privacy}","received_events_url":"https://api.github.com/users/astropy/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/astropy/astropy","description":"Repository for the Astropy core package","fork":false,"url":"https://api.github.com/repos/astropy/astropy","forks_url":"https://api.github.com/repos/astropy/astropy/forks","keys_url":"https://api.github.com/repos/astropy/astropy/keys{/key_id}","collaborators_url":"https://api.github.com/repos/astropy/astropy/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/astropy/astropy/teams","hooks_url":"https://api.github.com/repos/astropy/astropy/hooks","issue_events_url":"https://api.github.com/repos/astropy/astropy/issues/events{/number}","events_url":"https://api.github.com/repos/astropy/astropy/events","assignees_url":"https://api.github.com/repos/astropy/astropy/assignees{/user}","branches_url":"https://api.github.com/repos/astropy/astropy/branches{/branch}","tags_url":"https://api.github.com/repos/astropy/astropy/tags","blobs_url":"https://api.github.com/repos/astropy/astropy/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/astropy/astropy/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/astropy/astropy/git/refs{/sha}","trees_url":"https://api.github.com/repos/astropy/astropy/git/trees{/sha}","statuses_url":"https://api.github.com/repos/astropy/astropy/statuses/{sha}","languages_url":"https://api.github.com/repos/astropy/astropy/languages","stargazers_url":"https://api.github.com/repos/astropy/astropy/stargazers","contributors_url":"https://api.github.com/repos/astropy/astropy/contributors","subscribers_url":"https://api.github.com/repos/astropy/astropy/subscribers","subscription_url":"https://api.github.com/repos/astropy/astropy/subscription","commits_url":"https://api.github.com/repos/astropy/astropy/commits{/sha}","git_commits_url":"https://api.github.com/repos/astropy/astropy/git/commits{/sha}","comments_url":"https://api.github.com/repos/astropy/astropy/comments{/number}","issue_comment_url":"https://api.github.com/repos/astropy/astropy/issues/comments{/number}","contents_url":"https://api.github.com/repos/astropy/astropy/contents/{+path}","compare_url":"https://api.github.com/repos/astropy/astropy/compare/{base}...{head}","merges_url":"https://api.github.com/repos/astropy/astropy/merges","archive_url":"https://api.github.com/repos/astropy/astropy/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/astropy/astropy/downloads","issues_url":"https://api.github.com/repos/astropy/astropy/issues{/number}","pulls_url":"https://api.github.com/repos/astropy/astropy/pulls{/number}","milestones_url":"https://api.github.com/repos/astropy/astropy/milestones{/number}","notifications_url":"https://api.github.com/repos/astropy/astropy/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/astropy/astropy/labels{/name}","releases_url":"https://api.github.com/repos/astropy/astropy/releases{/id}","deployments_url":"https://api.github.com/repos/astropy/astropy/deployments","created_at":"2011-07-21T01:33:49Z","updated_at":"2017-11-28T14:13:21Z","pushed_at":"2017-11-29T02:22:21Z","git_url":"git://github.com/astropy/astropy.git","ssh_url":"git@github.com:astropy/astropy.git","clone_url":"https://github.com/astropy/astropy.git","svn_url":"https://github.com/astropy/astropy","homepage":"www.astropy.org","size":62744,"stargazers_count":1471,"watchers_count":1471,"language":"Python","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":780,"mirror_url":null,"archived":false,"open_issues_count":805,"license":{"key":"bsd-3-clause","name":"BSD 3-clause \"New\" or \"Revised\" License","spdx_id":"BSD-3-Clause","url":"https://api.github.com/licenses/bsd-3-clause"},"forks":780,"open_issues":805,"watchers":1471,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/astropy/astropy/pulls/6821"},"html":{"href":"https://github.com/astropy/astropy/pull/6821"},"issue":{"href":"https://api.github.com/repos/astropy/astropy/issues/6821"},"comments":{"href":"https://api.github.com/repos/astropy/astropy/issues/6821/comments"},"review_comments":{"href":"https://api.github.com/repos/astropy/astropy/pulls/6821/comments"},"review_comment":{"href":"https://api.github.com/repos/astropy/astropy/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/astropy/astropy/pulls/6821/commits"},"statuses":{"href":"https://api.github.com/repos/astropy/astropy/statuses/9ac94811efb4c36c14c44c2fabec21102573a8fd"}},"author_association":"OWNER"}}
{"actor":{"display_login":"astrofrog"}}