{"action":"created","comment":{"url":"https://api.github.com/repos/mapbox/rasterio/pulls/comments/44356118","id":44356118,"diff_hunk":"@@ -122,14 +122,26 @@ def mask(\n         bounds = geojson.get('bbox', calculate_bounds(geojson))\n \n         with rasterio.open(input) as src:\n-            has_disjoint_bounds = disjoint_bounds(bounds, src.bounds)\n+            # If y pixel value is positive, then invert y dimension in bounds\n+            invert_y = src.affine.e > 0\n+\n+            src_bounds = src.bounds\n+            if invert_y:\n+                src_bounds = [src.bounds[0], src.bounds[3],\n+                              src.bounds[2], src.bounds[1]]\n+","path":"rasterio/rio/features.py","position":12,"original_position":12,"commit_id":"15772d8d33b5041e018ab0c02756a3bafc189337","original_commit_id":"15772d8d33b5041e018ab0c02756a3bafc189337","user":{"login":"brendan-ward","id":3375604,"avatar_url":"https://avatars.githubusercontent.com/u/3375604?v=3","gravatar_id":"","url":"https://api.github.com/users/brendan-ward","html_url":"https://github.com/brendan-ward","followers_url":"https://api.github.com/users/brendan-ward/followers","following_url":"https://api.github.com/users/brendan-ward/following{/other_user}","gists_url":"https://api.github.com/users/brendan-ward/gists{/gist_id}","starred_url":"https://api.github.com/users/brendan-ward/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/brendan-ward/subscriptions","organizations_url":"https://api.github.com/users/brendan-ward/orgs","repos_url":"https://api.github.com/users/brendan-ward/repos","events_url":"https://api.github.com/users/brendan-ward/events{/privacy}","received_events_url":"https://api.github.com/users/brendan-ward/received_events","type":"User","site_admin":false},"body":"While working with NetCDF has turned my brain upside down, this case came from using a [test fixture raster](https://github.com/brendan-ward/rasterio/blob/overhaul_features_tests/tests/conftest.py#L192) with an `Affine.identity()` transform.\r\n\r\nI too thought a refactor of the core to handle that better would be preferred.  This was one of those cases where the issue, once found, begged a pretty immediate fix where I detected it, rather than introduce the scope creep of a larger refactor.\r\n\r\nWe could either cut this out here and refactor that test fixture (possibly touching a bunch of tests along the way), or leave as a temporary shim and add an issue to address the problem in a more robust fashion.  Preference?","created_at":"2015-11-10T00:42:58Z","updated_at":"2015-11-10T00:42:58Z","html_url":"https://github.com/mapbox/rasterio/pull/522#discussion_r44356118","pull_request_url":"https://api.github.com/repos/mapbox/rasterio/pulls/522","_links":{"self":{"href":"https://api.github.com/repos/mapbox/rasterio/pulls/comments/44356118"},"html":{"href":"https://github.com/mapbox/rasterio/pull/522#discussion_r44356118"},"pull_request":{"href":"https://api.github.com/repos/mapbox/rasterio/pulls/522"}}},"pull_request":{"url":"https://api.github.com/repos/mapbox/rasterio/pulls/522","id":50085430,"html_url":"https://github.com/mapbox/rasterio/pull/522","diff_url":"https://github.com/mapbox/rasterio/pull/522.diff","patch_url":"https://github.com/mapbox/rasterio/pull/522.patch","issue_url":"https://api.github.com/repos/mapbox/rasterio/issues/522","number":522,"state":"open","locked":false,"title":"Refactor of features tests","user":{"login":"brendan-ward","id":3375604,"avatar_url":"https://avatars.githubusercontent.com/u/3375604?v=3","gravatar_id":"","url":"https://api.github.com/users/brendan-ward","html_url":"https://github.com/brendan-ward","followers_url":"https://api.github.com/users/brendan-ward/followers","following_url":"https://api.github.com/users/brendan-ward/following{/other_user}","gists_url":"https://api.github.com/users/brendan-ward/gists{/gist_id}","starred_url":"https://api.github.com/users/brendan-ward/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/brendan-ward/subscriptions","organizations_url":"https://api.github.com/users/brendan-ward/orgs","repos_url":"https://api.github.com/users/brendan-ward/repos","events_url":"https://api.github.com/users/brendan-ward/events{/privacy}","received_events_url":"https://api.github.com/users/brendan-ward/received_events","type":"User","site_admin":false},"body":"This PR involves a major refactor of the tests for `features.py` and `rio/features.py`.\r\n\r\nGoals:\r\n1. Make tests as small, simple, and fast as possible\r\n2. Use pytest fixtures for repeated inputs\r\n3. Increase test coverage\r\n\r\nAlong the way, I dropped WKB support from `_features.pyx` (wasn't used) and moved as much of the validation logic for `shapes` and `sieve`  as possible from the Python functions (wrappers) to the base Cython implementations to keep these more tightly encapsulated.  `rasterize` does too much in the Python layer to easily move into the Cython implementation.  I also refactored the validation logic for data types used by rasterize into function in `dtypes` to enhance accessibility and testability.\r\n\r\nI also consolidated the several individual files with tests for various parts of `features` into a single test file; makes it somewhat easier to manage when there is one test file to one source file.\r\n\r\nI used pytest fixtures heavily to use very small test images and files; this contributed a significant speed increase to the test suite - toward #450.\r\n\r\nI found and fixed a few isolated bugs along the way.\r\n\r\nI made a couple things a bit more strict:\r\n1) `bounds` does not return `None` on exception, it now just passes that exception through.\r\n2) `rasterize` no longer ignores invalid shapes; they will raise an exception instead.\r\n\r\nNote: the decrease in coverage is due to mis-reporting of coverage in `_features.pyx` and `rio/features.py` files; much more of the functionality is actually covered than is indicated here.  I believe that actual coverage has actually increased over the prior implementation.\r\n\r\nI've found awkward behavior with `coverage` that is proving hard to explain especially for the CLI tests; if I run the same call to the `click` test runner 2x in the same test function, I get coverage but if I run it only once, I don't.  This is partly why coverage was recorded higher previously, because the test functions before did everything and the kitchen sink in a single function.","created_at":"2015-11-09T05:07:09Z","updated_at":"2015-11-10T00:42:58Z","closed_at":null,"merged_at":null,"merge_commit_sha":"65f8f1d5b75058409de0430867eead931962e858","assignee":null,"milestone":{"url":"https://api.github.com/repos/mapbox/rasterio/milestones/35","html_url":"https://github.com/mapbox/rasterio/milestones/pre%201.0","labels_url":"https://api.github.com/repos/mapbox/rasterio/milestones/35/labels","id":1221914,"number":35,"title":"pre 1.0","description":null,"creator":{"login":"sgillies","id":33697,"avatar_url":"https://avatars.githubusercontent.com/u/33697?v=3","gravatar_id":"","url":"https://api.github.com/users/sgillies","html_url":"https://github.com/sgillies","followers_url":"https://api.github.com/users/sgillies/followers","following_url":"https://api.github.com/users/sgillies/following{/other_user}","gists_url":"https://api.github.com/users/sgillies/gists{/gist_id}","starred_url":"https://api.github.com/users/sgillies/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sgillies/subscriptions","organizations_url":"https://api.github.com/users/sgillies/orgs","repos_url":"https://api.github.com/users/sgillies/repos","events_url":"https://api.github.com/users/sgillies/events{/privacy}","received_events_url":"https://api.github.com/users/sgillies/received_events","type":"User","site_admin":false},"open_issues":7,"closed_issues":16,"state":"open","created_at":"2015-07-23T11:40:52Z","updated_at":"2015-11-09T22:40:43Z","due_on":null,"closed_at":null},"commits_url":"https://api.github.com/repos/mapbox/rasterio/pulls/522/commits","review_comments_url":"https://api.github.com/repos/mapbox/rasterio/pulls/522/comments","review_comment_url":"https://api.github.com/repos/mapbox/rasterio/pulls/comments{/number}","comments_url":"https://api.github.com/repos/mapbox/rasterio/issues/522/comments","statuses_url":"https://api.github.com/repos/mapbox/rasterio/statuses/15772d8d33b5041e018ab0c02756a3bafc189337","head":{"label":"brendan-ward:overhaul_features_tests","ref":"overhaul_features_tests","sha":"15772d8d33b5041e018ab0c02756a3bafc189337","user":{"login":"brendan-ward","id":3375604,"avatar_url":"https://avatars.githubusercontent.com/u/3375604?v=3","gravatar_id":"","url":"https://api.github.com/users/brendan-ward","html_url":"https://github.com/brendan-ward","followers_url":"https://api.github.com/users/brendan-ward/followers","following_url":"https://api.github.com/users/brendan-ward/following{/other_user}","gists_url":"https://api.github.com/users/brendan-ward/gists{/gist_id}","starred_url":"https://api.github.com/users/brendan-ward/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/brendan-ward/subscriptions","organizations_url":"https://api.github.com/users/brendan-ward/orgs","repos_url":"https://api.github.com/users/brendan-ward/repos","events_url":"https://api.github.com/users/brendan-ward/events{/privacy}","received_events_url":"https://api.github.com/users/brendan-ward/received_events","type":"User","site_admin":false},"repo":{"id":24509634,"name":"rasterio","full_name":"brendan-ward/rasterio","owner":{"login":"brendan-ward","id":3375604,"avatar_url":"https://avatars.githubusercontent.com/u/3375604?v=3","gravatar_id":"","url":"https://api.github.com/users/brendan-ward","html_url":"https://github.com/brendan-ward","followers_url":"https://api.github.com/users/brendan-ward/followers","following_url":"https://api.github.com/users/brendan-ward/following{/other_user}","gists_url":"https://api.github.com/users/brendan-ward/gists{/gist_id}","starred_url":"https://api.github.com/users/brendan-ward/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/brendan-ward/subscriptions","organizations_url":"https://api.github.com/users/brendan-ward/orgs","repos_url":"https://api.github.com/users/brendan-ward/repos","events_url":"https://api.github.com/users/brendan-ward/events{/privacy}","received_events_url":"https://api.github.com/users/brendan-ward/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/brendan-ward/rasterio","description":"Not the authoritative repo, see https://github.com/mapbox/rasterio","fork":true,"url":"https://api.github.com/repos/brendan-ward/rasterio","forks_url":"https://api.github.com/repos/brendan-ward/rasterio/forks","keys_url":"https://api.github.com/repos/brendan-ward/rasterio/keys{/key_id}","collaborators_url":"https://api.github.com/repos/brendan-ward/rasterio/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/brendan-ward/rasterio/teams","hooks_url":"https://api.github.com/repos/brendan-ward/rasterio/hooks","issue_events_url":"https://api.github.com/repos/brendan-ward/rasterio/issues/events{/number}","events_url":"https://api.github.com/repos/brendan-ward/rasterio/events","assignees_url":"https://api.github.com/repos/brendan-ward/rasterio/assignees{/user}","branches_url":"https://api.github.com/repos/brendan-ward/rasterio/branches{/branch}","tags_url":"https://api.github.com/repos/brendan-ward/rasterio/tags","blobs_url":"https://api.github.com/repos/brendan-ward/rasterio/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/brendan-ward/rasterio/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/brendan-ward/rasterio/git/refs{/sha}","trees_url":"https://api.github.com/repos/brendan-ward/rasterio/git/trees{/sha}","statuses_url":"https://api.github.com/repos/brendan-ward/rasterio/statuses/{sha}","languages_url":"https://api.github.com/repos/brendan-ward/rasterio/languages","stargazers_url":"https://api.github.com/repos/brendan-ward/rasterio/stargazers","contributors_url":"https://api.github.com/repos/brendan-ward/rasterio/contributors","subscribers_url":"https://api.github.com/repos/brendan-ward/rasterio/subscribers","subscription_url":"https://api.github.com/repos/brendan-ward/rasterio/subscription","commits_url":"https://api.github.com/repos/brendan-ward/rasterio/commits{/sha}","git_commits_url":"https://api.github.com/repos/brendan-ward/rasterio/git/commits{/sha}","comments_url":"https://api.github.com/repos/brendan-ward/rasterio/comments{/number}","issue_comment_url":"https://api.github.com/repos/brendan-ward/rasterio/issues/comments{/number}","contents_url":"https://api.github.com/repos/brendan-ward/rasterio/contents/{+path}","compare_url":"https://api.github.com/repos/brendan-ward/rasterio/compare/{base}...{head}","merges_url":"https://api.github.com/repos/brendan-ward/rasterio/merges","archive_url":"https://api.github.com/repos/brendan-ward/rasterio/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/brendan-ward/rasterio/downloads","issues_url":"https://api.github.com/repos/brendan-ward/rasterio/issues{/number}","pulls_url":"https://api.github.com/repos/brendan-ward/rasterio/pulls{/number}","milestones_url":"https://api.github.com/repos/brendan-ward/rasterio/milestones{/number}","notifications_url":"https://api.github.com/repos/brendan-ward/rasterio/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/brendan-ward/rasterio/labels{/name}","releases_url":"https://api.github.com/repos/brendan-ward/rasterio/releases{/id}","created_at":"2014-09-26T17:55:19Z","updated_at":"2015-04-25T03:42:35Z","pushed_at":"2015-11-09T04:40:27Z","git_url":"git://github.com/brendan-ward/rasterio.git","ssh_url":"git@github.com:brendan-ward/rasterio.git","clone_url":"https://github.com/brendan-ward/rasterio.git","svn_url":"https://github.com/brendan-ward/rasterio","homepage":"","size":3526,"stargazers_count":0,"watchers_count":0,"language":"Python","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"mapbox:master","ref":"master","sha":"718b8386e43efa521288a661c262c5e472a060fe","user":{"login":"mapbox","id":600935,"avatar_url":"https://avatars.githubusercontent.com/u/600935?v=3","gravatar_id":"","url":"https://api.github.com/users/mapbox","html_url":"https://github.com/mapbox","followers_url":"https://api.github.com/users/mapbox/followers","following_url":"https://api.github.com/users/mapbox/following{/other_user}","gists_url":"https://api.github.com/users/mapbox/gists{/gist_id}","starred_url":"https://api.github.com/users/mapbox/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mapbox/subscriptions","organizations_url":"https://api.github.com/users/mapbox/orgs","repos_url":"https://api.github.com/users/mapbox/repos","events_url":"https://api.github.com/users/mapbox/events{/privacy}","received_events_url":"https://api.github.com/users/mapbox/received_events","type":"Organization","site_admin":false},"repo":{"id":14116604,"name":"rasterio","full_name":"mapbox/rasterio","owner":{"login":"mapbox","id":600935,"avatar_url":"https://avatars.githubusercontent.com/u/600935?v=3","gravatar_id":"","url":"https://api.github.com/users/mapbox","html_url":"https://github.com/mapbox","followers_url":"https://api.github.com/users/mapbox/followers","following_url":"https://api.github.com/users/mapbox/following{/other_user}","gists_url":"https://api.github.com/users/mapbox/gists{/gist_id}","starred_url":"https://api.github.com/users/mapbox/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mapbox/subscriptions","organizations_url":"https://api.github.com/users/mapbox/orgs","repos_url":"https://api.github.com/users/mapbox/repos","events_url":"https://api.github.com/users/mapbox/events{/privacy}","received_events_url":"https://api.github.com/users/mapbox/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/mapbox/rasterio","description":"Clean and fast and geospatial raster I/O for Python programmers who use Numpy","fork":false,"url":"https://api.github.com/repos/mapbox/rasterio","forks_url":"https://api.github.com/repos/mapbox/rasterio/forks","keys_url":"https://api.github.com/repos/mapbox/rasterio/keys{/key_id}","collaborators_url":"https://api.github.com/repos/mapbox/rasterio/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/mapbox/rasterio/teams","hooks_url":"https://api.github.com/repos/mapbox/rasterio/hooks","issue_events_url":"https://api.github.com/repos/mapbox/rasterio/issues/events{/number}","events_url":"https://api.github.com/repos/mapbox/rasterio/events","assignees_url":"https://api.github.com/repos/mapbox/rasterio/assignees{/user}","branches_url":"https://api.github.com/repos/mapbox/rasterio/branches{/branch}","tags_url":"https://api.github.com/repos/mapbox/rasterio/tags","blobs_url":"https://api.github.com/repos/mapbox/rasterio/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/mapbox/rasterio/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/mapbox/rasterio/git/refs{/sha}","trees_url":"https://api.github.com/repos/mapbox/rasterio/git/trees{/sha}","statuses_url":"https://api.github.com/repos/mapbox/rasterio/statuses/{sha}","languages_url":"https://api.github.com/repos/mapbox/rasterio/languages","stargazers_url":"https://api.github.com/repos/mapbox/rasterio/stargazers","contributors_url":"https://api.github.com/repos/mapbox/rasterio/contributors","subscribers_url":"https://api.github.com/repos/mapbox/rasterio/subscribers","subscription_url":"https://api.github.com/repos/mapbox/rasterio/subscription","commits_url":"https://api.github.com/repos/mapbox/rasterio/commits{/sha}","git_commits_url":"https://api.github.com/repos/mapbox/rasterio/git/commits{/sha}","comments_url":"https://api.github.com/repos/mapbox/rasterio/comments{/number}","issue_comment_url":"https://api.github.com/repos/mapbox/rasterio/issues/comments{/number}","contents_url":"https://api.github.com/repos/mapbox/rasterio/contents/{+path}","compare_url":"https://api.github.com/repos/mapbox/rasterio/compare/{base}...{head}","merges_url":"https://api.github.com/repos/mapbox/rasterio/merges","archive_url":"https://api.github.com/repos/mapbox/rasterio/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/mapbox/rasterio/downloads","issues_url":"https://api.github.com/repos/mapbox/rasterio/issues{/number}","pulls_url":"https://api.github.com/repos/mapbox/rasterio/pulls{/number}","milestones_url":"https://api.github.com/repos/mapbox/rasterio/milestones{/number}","notifications_url":"https://api.github.com/repos/mapbox/rasterio/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/mapbox/rasterio/labels{/name}","releases_url":"https://api.github.com/repos/mapbox/rasterio/releases{/id}","created_at":"2013-11-04T16:36:27Z","updated_at":"2015-11-09T22:20:35Z","pushed_at":"2015-11-09T22:23:37Z","git_url":"git://github.com/mapbox/rasterio.git","ssh_url":"git@github.com:mapbox/rasterio.git","clone_url":"https://github.com/mapbox/rasterio.git","svn_url":"https://github.com/mapbox/rasterio","homepage":"","size":7331,"stargazers_count":321,"watchers_count":321,"language":"Python","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":true,"forks_count":69,"mirror_url":null,"open_issues_count":48,"forks":69,"open_issues":48,"watchers":321,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/mapbox/rasterio/pulls/522"},"html":{"href":"https://github.com/mapbox/rasterio/pull/522"},"issue":{"href":"https://api.github.com/repos/mapbox/rasterio/issues/522"},"comments":{"href":"https://api.github.com/repos/mapbox/rasterio/issues/522/comments"},"review_comments":{"href":"https://api.github.com/repos/mapbox/rasterio/pulls/522/comments"},"review_comment":{"href":"https://api.github.com/repos/mapbox/rasterio/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/mapbox/rasterio/pulls/522/commits"},"statuses":{"href":"https://api.github.com/repos/mapbox/rasterio/statuses/15772d8d33b5041e018ab0c02756a3bafc189337"}}}}