{"action":"created","issue":{"url":"https://api.github.com/repos/esheldon/fitsio/issues/9","labels_url":"https://api.github.com/repos/esheldon/fitsio/issues/9/labels{/name}","comments_url":"https://api.github.com/repos/esheldon/fitsio/issues/9/comments","events_url":"https://api.github.com/repos/esheldon/fitsio/issues/9/events","html_url":"https://github.com/esheldon/fitsio/pull/9","id":16149946,"number":9,"title":"Some improvements","user":{"login":"rainwoodman","id":138060,"avatar_url":"https://avatars.githubusercontent.com/u/138060?v=3","gravatar_id":"","url":"https://api.github.com/users/rainwoodman","html_url":"https://github.com/rainwoodman","followers_url":"https://api.github.com/users/rainwoodman/followers","following_url":"https://api.github.com/users/rainwoodman/following{/other_user}","gists_url":"https://api.github.com/users/rainwoodman/gists{/gist_id}","starred_url":"https://api.github.com/users/rainwoodman/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rainwoodman/subscriptions","organizations_url":"https://api.github.com/users/rainwoodman/orgs","repos_url":"https://api.github.com/users/rainwoodman/repos","events_url":"https://api.github.com/users/rainwoodman/events{/privacy}","received_events_url":"https://api.github.com/users/rainwoodman/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":7,"created_at":"2013-06-28T16:31:21Z","updated_at":"2015-03-31T20:06:51Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/esheldon/fitsio/pulls/9","html_url":"https://github.com/esheldon/fitsio/pull/9","diff_url":"https://github.com/esheldon/fitsio/pull/9.diff","patch_url":"https://github.com/esheldon/fitsio/pull/9.patch"},"body":"Trying to read the SDSS spectra files with fitsio and noticed a few weirdness:\r\n\r\nThe DR10 Spectra's doesn't have Z_PERSON. But in the repr of fits[2]['Z_PERSON'] \r\nI see subcolumn 'Z'.\r\nThe first commit fixes this. Is is because when ColumnSubset.columns is scalar and improperly handled.\r\n\r\nThe second commit fixes a core dump when I run\r\nfits[2]['CLASS_PERSON']['CLASS'][:]\r\nIt is because when the target is a scalar, strides is NULL and PyArray_STRIDE, core dumps. The fact that array is a numpy scalar is probably also worrisome; but with this modification the code gives the correct result 'QSO' without core dumps.\r\n\r\nAnother commit disables printing to stdout. Because printing to stdout from a library will mess up with regular outputs from the programs using the library. issuing python warnings might be more appropriate.\r\n\r\nAnother commit disables explicit relative imports. Python by default looks for modules in . first, thus there is no need to explicitly declear '.' in the imports.\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/esheldon/fitsio/issues/comments/88229413","html_url":"https://github.com/esheldon/fitsio/pull/9#issuecomment-88229413","issue_url":"https://api.github.com/repos/esheldon/fitsio/issues/9","id":88229413,"user":{"login":"rainwoodman","id":138060,"avatar_url":"https://avatars.githubusercontent.com/u/138060?v=3","gravatar_id":"","url":"https://api.github.com/users/rainwoodman","html_url":"https://github.com/rainwoodman","followers_url":"https://api.github.com/users/rainwoodman/followers","following_url":"https://api.github.com/users/rainwoodman/following{/other_user}","gists_url":"https://api.github.com/users/rainwoodman/gists{/gist_id}","starred_url":"https://api.github.com/users/rainwoodman/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rainwoodman/subscriptions","organizations_url":"https://api.github.com/users/rainwoodman/orgs","repos_url":"https://api.github.com/users/rainwoodman/repos","events_url":"https://api.github.com/users/rainwoodman/events{/privacy}","received_events_url":"https://api.github.com/users/rainwoodman/received_events","type":"User","site_admin":false},"created_at":"2015-03-31T20:06:51Z","updated_at":"2015-03-31T20:06:51Z","body":"I agree the example I gave is indeed too simple. A typical useful case where running multiple jobs become cumbersome is\r\n\r\n```\r\nfilename = ['a.fits', 'b.fits', 'c.fits']\r\nimport multiprocessing.pool\r\npool = multiprocessing.pool.ThreadPool()\r\ndef process(filename)\r\n    f = fitsio.FITS(filename)\r\n    # process file f\r\n    # for example select objects above a threshold\r\n    return list_of_objects\r\n\r\nobjects = numpy.concatenate(pool.map(filenames))\r\n```\r\n\r\nI do not agree with you this patch makes the code harder to read, write or understand.\r\nThe NOGIL instruction is similar to the \"with nogil:\" construct in Cython. It clearly states the purpose: functionality that does not involve python runtime shall not hold the GIL. The flow of the original source code is unaffected.\r\n\r\nI will start a new PR with the test cases. \r\n\r\n\r\n"}}