{"action":"created","comment":{"url":"https://api.github.com/repos/metoppv/improver/pulls/comments/184434309","pull_request_review_id":115631239,"id":184434309,"diff_hunk":"@@ -454,165 +454,216 @@ def process(self, cube, weights=None):\n                         one value. Returns the original cube in this case.\n \n         \"\"\"\n+\n+        self._check_input_cube_is_iris_cube(cube)\n+\n+        # if the coordinate being blended is forecast_reference_time.\n+        if self.coord == \"forecast_reference_time\":\n+            self._check_all_time_points_equal(cube)\n+\n+        if weights is not None:\n+            self._check_weights_have_coord_shape(cube, weights)\n+\n+        # only do blending if coordinate is a dimension coordinate\n+        coord_dim = cube.coord_dims(self.coord)\n+        if coord_dim:\n+            result = self._perform_blending(cube, weights)\n+        else:\n+            # scalar coordinate\n+            msg = ('Trying to blend across a scalar coordinate with only one'\n+                   ' value. Returning original cube')\n+            warnings.warn(msg)\n+            result = cube\n+\n+        # adjust values of collapsed coordinates if required\n+        if self.coord_adjust is not None:\n+            result = self._adjust_coord(result, cube, coord_dim)\n+\n+        return result\n+\n+    @staticmethod\n+    def _check_input_cube_is_iris_cube(cube):\n         if not isinstance(cube, iris.cube.Cube):\n             msg = ('The first argument must be an instance of '\n                    'iris.cube.Cube but is'\n                    ' {0:s}.'.format(type(cube)))\n             raise TypeError(msg)\n \n-        # Check that the points within the time coordinate are equal\n-        # if the coordinate being blended is forecast_reference_time.\n-        if self.coord == \"forecast_reference_time\":\n-            if cube.coords(\"time\"):\n-                time_points = cube.coord(\"time\").points\n-                if len(np.unique(time_points)) > 1:\n-                    msg = (\"For blending using the forecast_reference_time \"\n-                           \"coordinate, the points within the time coordinate \"\n-                           \"need to be the same. The time points within the \"\n-                           \"input cube are {}\".format(time_points))\n-                    raise ValueError(msg)\n-\n-        # Check to see if the data is percentile data\n+    @staticmethod\n+    def _check_all_time_points_equal(cube):\n+        if cube.coords(\"time\"):\n+            time_points = cube.coord(\"time\").points\n+            if len(np.unique(time_points)) > 1:\n+                msg = (\"For blending using the forecast_reference_time \"\n+                       \"coordinate, the points within the time coordinate \"\n+                       \"need to be the same. The time points within the \"\n+                       \"input cube are {}\".format(time_points))\n+                raise ValueError(msg)\n+\n+    def _check_weights_have_coord_shape(self, cube, weights):\n+        if np.array(weights).shape != cube.coord(self.coord).points.shape:\n+            msg = ('The weights array must match the shape '\n+                   'of the coordinate in the input cube; '\n+                   'weight shape is '\n+                   '{0:s}'.format(np.array(weights).shape) +\n+                   ', cube shape is '\n+                   '{0:s}'.format(cube.coord(self.coord).points.shape))\n+            raise ValueError(msg)\n+\n+    def _perform_blending(self, cube, weights):\n+        cubelist = iris.cube.CubeList([])\n+        for cube_thres in self._slices_over_threshold(cube):\n+            cube_new = self._perform_blend_across_threshold(cube_thres,\n+                                                            weights)\n+            cube_new = conform_metadata(\n+                cube_new, cube_thres, coord=self.coord,\n+                cycletime=self.cycletime,\n+                coords_for_bounds_removal=self.coords_for_bounds_removal)\n+\n+            cubelist.append(cube_new)\n+        result = cubelist.merge_cube()\n+        if isinstance(cubelist[0].data, np.ma.core.MaskedArray):\n+            result.data = np.ma.array(result.data)\n+        return result\n+\n+    def _perform_blend_across_threshold(self, cube_thres, weights):\n+\n+        # check to see if the data is percentile data\n+        # if not, perc_coord will be None\n+        perc_coord = self._get_perc_dim_coord(cube_thres)\n+\n+        # Blend the cube across the coordinate\n+        if self.mode == \"weighted_mean\":\n+            # Use percentile Aggregator if required\n+            if perc_coord:\n+                cube_new = self._blend_weighted_mean_percentile(cube_thres,\n+                                                                weights,\n+                                                                perc_coord)\n+            # Else do a simple weighted average\n+            else:\n+                cube_new = self._blend_weighted_mean(cube_thres, weights)\n+\n+        # Else use the maximum probability aggregator.\n+        elif self.mode == \"weighted_maximum\":\n+            # If we have a percentile dimension and the mode is 'max' raise an\n+            # exception.\n+            if perc_coord:\n+                msg = ('The \"weighted_maximum\" mode cannot be used with'\n+                       ' percentile data.')\n+                raise ValueError(msg)\n+            else:","path":"lib/improver/blending/weighted_blend.py","position":114,"original_position":114,"commit_id":"e4c3fc3410a79f5af87ed1f23a449c1bf62f4b82","original_commit_id":"e4c3fc3410a79f5af87ed1f23a449c1bf62f4b82","user":{"login":"arh89","id":4810226,"avatar_url":"https://avatars0.githubusercontent.com/u/4810226?v=4","gravatar_id":"","url":"https://api.github.com/users/arh89","html_url":"https://github.com/arh89","followers_url":"https://api.github.com/users/arh89/followers","following_url":"https://api.github.com/users/arh89/following{/other_user}","gists_url":"https://api.github.com/users/arh89/gists{/gist_id}","starred_url":"https://api.github.com/users/arh89/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/arh89/subscriptions","organizations_url":"https://api.github.com/users/arh89/orgs","repos_url":"https://api.github.com/users/arh89/repos","events_url":"https://api.github.com/users/arh89/events{/privacy}","received_events_url":"https://api.github.com/users/arh89/received_events","type":"User","site_admin":false},"body":"I agree. Do we have an overall IMPROVER style for this type of stuff?\r\nPersonally, I like to know which branch I've gone into, rather than having to track down any early exit etc.. but removing `else` would reduce the complexity, so I'm happy to remove this.","created_at":"2018-04-26T15:35:00Z","updated_at":"2018-04-26T15:35:01Z","html_url":"https://github.com/metoppv/improver/pull/520#discussion_r184434309","pull_request_url":"https://api.github.com/repos/metoppv/improver/pulls/520","author_association":"CONTRIBUTOR","_links":{"self":{"href":"https://api.github.com/repos/metoppv/improver/pulls/comments/184434309"},"html":{"href":"https://github.com/metoppv/improver/pull/520#discussion_r184434309"},"pull_request":{"href":"https://api.github.com/repos/metoppv/improver/pulls/520"}},"in_reply_to_id":184371332},"pull_request":{"url":"https://api.github.com/repos/metoppv/improver/pulls/520","id":183749241,"html_url":"https://github.com/metoppv/improver/pull/520","diff_url":"https://github.com/metoppv/improver/pull/520.diff","patch_url":"https://github.com/metoppv/improver/pull/520.patch","issue_url":"https://api.github.com/repos/metoppv/improver/issues/520","number":520,"state":"open","locked":false,"title":"Refactor weighted_blend.py","user":{"login":"arh89","id":4810226,"avatar_url":"https://avatars0.githubusercontent.com/u/4810226?v=4","gravatar_id":"","url":"https://api.github.com/users/arh89","html_url":"https://github.com/arh89","followers_url":"https://api.github.com/users/arh89/followers","following_url":"https://api.github.com/users/arh89/following{/other_user}","gists_url":"https://api.github.com/users/arh89/gists{/gist_id}","starred_url":"https://api.github.com/users/arh89/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/arh89/subscriptions","organizations_url":"https://api.github.com/users/arh89/orgs","repos_url":"https://api.github.com/users/arh89/repos","events_url":"https://api.github.com/users/arh89/events{/privacy}","received_events_url":"https://api.github.com/users/arh89/received_events","type":"User","site_admin":false},"body":"This is just an attempt to break up the complex `process` method in `WeightedBlendAcrossWholeDimension` into smaller chunks. No docstrings have been added to the new methods because I wanted to put this up for others to see before I put the effort into adding these.\r\nWe can still go further (e.g: moving the definition of the weights to where we check that we have the correct number), but I didn't want to do too much in one go.\r\n\r\nTesting:\r\n - [x] Ran tests and they passed OK","created_at":"2018-04-24T14:08:23Z","updated_at":"2018-04-26T15:35:01Z","closed_at":null,"merged_at":null,"merge_commit_sha":"db3e3f38f7ff5c029a4889ac165a52534483a30a","assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"labels":[],"milestone":null,"commits_url":"https://api.github.com/repos/metoppv/improver/pulls/520/commits","review_comments_url":"https://api.github.com/repos/metoppv/improver/pulls/520/comments","review_comment_url":"https://api.github.com/repos/metoppv/improver/pulls/comments{/number}","comments_url":"https://api.github.com/repos/metoppv/improver/issues/520/comments","statuses_url":"https://api.github.com/repos/metoppv/improver/statuses/e4c3fc3410a79f5af87ed1f23a449c1bf62f4b82","head":{"label":"arh89:weighted_blend_refactor","ref":"weighted_blend_refactor","sha":"e4c3fc3410a79f5af87ed1f23a449c1bf62f4b82","user":{"login":"arh89","id":4810226,"avatar_url":"https://avatars0.githubusercontent.com/u/4810226?v=4","gravatar_id":"","url":"https://api.github.com/users/arh89","html_url":"https://github.com/arh89","followers_url":"https://api.github.com/users/arh89/followers","following_url":"https://api.github.com/users/arh89/following{/other_user}","gists_url":"https://api.github.com/users/arh89/gists{/gist_id}","starred_url":"https://api.github.com/users/arh89/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/arh89/subscriptions","organizations_url":"https://api.github.com/users/arh89/orgs","repos_url":"https://api.github.com/users/arh89/repos","events_url":"https://api.github.com/users/arh89/events{/privacy}","received_events_url":"https://api.github.com/users/arh89/received_events","type":"User","site_admin":false},"repo":{"id":91713613,"name":"improver","full_name":"arh89/improver","owner":{"login":"arh89","id":4810226,"avatar_url":"https://avatars0.githubusercontent.com/u/4810226?v=4","gravatar_id":"","url":"https://api.github.com/users/arh89","html_url":"https://github.com/arh89","followers_url":"https://api.github.com/users/arh89/followers","following_url":"https://api.github.com/users/arh89/following{/other_user}","gists_url":"https://api.github.com/users/arh89/gists{/gist_id}","starred_url":"https://api.github.com/users/arh89/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/arh89/subscriptions","organizations_url":"https://api.github.com/users/arh89/orgs","repos_url":"https://api.github.com/users/arh89/repos","events_url":"https://api.github.com/users/arh89/events{/privacy}","received_events_url":"https://api.github.com/users/arh89/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/arh89/improver","description":"IMPROVER","fork":true,"url":"https://api.github.com/repos/arh89/improver","forks_url":"https://api.github.com/repos/arh89/improver/forks","keys_url":"https://api.github.com/repos/arh89/improver/keys{/key_id}","collaborators_url":"https://api.github.com/repos/arh89/improver/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/arh89/improver/teams","hooks_url":"https://api.github.com/repos/arh89/improver/hooks","issue_events_url":"https://api.github.com/repos/arh89/improver/issues/events{/number}","events_url":"https://api.github.com/repos/arh89/improver/events","assignees_url":"https://api.github.com/repos/arh89/improver/assignees{/user}","branches_url":"https://api.github.com/repos/arh89/improver/branches{/branch}","tags_url":"https://api.github.com/repos/arh89/improver/tags","blobs_url":"https://api.github.com/repos/arh89/improver/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/arh89/improver/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/arh89/improver/git/refs{/sha}","trees_url":"https://api.github.com/repos/arh89/improver/git/trees{/sha}","statuses_url":"https://api.github.com/repos/arh89/improver/statuses/{sha}","languages_url":"https://api.github.com/repos/arh89/improver/languages","stargazers_url":"https://api.github.com/repos/arh89/improver/stargazers","contributors_url":"https://api.github.com/repos/arh89/improver/contributors","subscribers_url":"https://api.github.com/repos/arh89/improver/subscribers","subscription_url":"https://api.github.com/repos/arh89/improver/subscription","commits_url":"https://api.github.com/repos/arh89/improver/commits{/sha}","git_commits_url":"https://api.github.com/repos/arh89/improver/git/commits{/sha}","comments_url":"https://api.github.com/repos/arh89/improver/comments{/number}","issue_comment_url":"https://api.github.com/repos/arh89/improver/issues/comments{/number}","contents_url":"https://api.github.com/repos/arh89/improver/contents/{+path}","compare_url":"https://api.github.com/repos/arh89/improver/compare/{base}...{head}","merges_url":"https://api.github.com/repos/arh89/improver/merges","archive_url":"https://api.github.com/repos/arh89/improver/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/arh89/improver/downloads","issues_url":"https://api.github.com/repos/arh89/improver/issues{/number}","pulls_url":"https://api.github.com/repos/arh89/improver/pulls{/number}","milestones_url":"https://api.github.com/repos/arh89/improver/milestones{/number}","notifications_url":"https://api.github.com/repos/arh89/improver/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/arh89/improver/labels{/name}","releases_url":"https://api.github.com/repos/arh89/improver/releases{/id}","deployments_url":"https://api.github.com/repos/arh89/improver/deployments","created_at":"2017-05-18T16:10:24Z","updated_at":"2018-04-22T23:01:29Z","pushed_at":"2018-04-26T09:13:15Z","git_url":"git://github.com/arh89/improver.git","ssh_url":"git@github.com:arh89/improver.git","clone_url":"https://github.com/arh89/improver.git","svn_url":"https://github.com/arh89/improver","homepage":null,"size":3324,"stargazers_count":0,"watchers_count":0,"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":1,"license":{"key":"other","name":"Other","spdx_id":null,"url":null},"forks":0,"open_issues":1,"watchers":0,"default_branch":"master"}},"base":{"label":"metoppv:master","ref":"master","sha":"c5ad16d7f5c710eca303a29bb54de37875289677","user":{"login":"metoppv","id":26465485,"avatar_url":"https://avatars2.githubusercontent.com/u/26465485?v=4","gravatar_id":"","url":"https://api.github.com/users/metoppv","html_url":"https://github.com/metoppv","followers_url":"https://api.github.com/users/metoppv/followers","following_url":"https://api.github.com/users/metoppv/following{/other_user}","gists_url":"https://api.github.com/users/metoppv/gists{/gist_id}","starred_url":"https://api.github.com/users/metoppv/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/metoppv/subscriptions","organizations_url":"https://api.github.com/users/metoppv/orgs","repos_url":"https://api.github.com/users/metoppv/repos","events_url":"https://api.github.com/users/metoppv/events{/privacy}","received_events_url":"https://api.github.com/users/metoppv/received_events","type":"Organization","site_admin":false},"repo":{"id":85334761,"name":"improver","full_name":"metoppv/improver","owner":{"login":"metoppv","id":26465485,"avatar_url":"https://avatars2.githubusercontent.com/u/26465485?v=4","gravatar_id":"","url":"https://api.github.com/users/metoppv","html_url":"https://github.com/metoppv","followers_url":"https://api.github.com/users/metoppv/followers","following_url":"https://api.github.com/users/metoppv/following{/other_user}","gists_url":"https://api.github.com/users/metoppv/gists{/gist_id}","starred_url":"https://api.github.com/users/metoppv/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/metoppv/subscriptions","organizations_url":"https://api.github.com/users/metoppv/orgs","repos_url":"https://api.github.com/users/metoppv/repos","events_url":"https://api.github.com/users/metoppv/events{/privacy}","received_events_url":"https://api.github.com/users/metoppv/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/metoppv/improver","description":"IMPROVER is a library of algorithms for meteorological post-processing and verification.","fork":false,"url":"https://api.github.com/repos/metoppv/improver","forks_url":"https://api.github.com/repos/metoppv/improver/forks","keys_url":"https://api.github.com/repos/metoppv/improver/keys{/key_id}","collaborators_url":"https://api.github.com/repos/metoppv/improver/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/metoppv/improver/teams","hooks_url":"https://api.github.com/repos/metoppv/improver/hooks","issue_events_url":"https://api.github.com/repos/metoppv/improver/issues/events{/number}","events_url":"https://api.github.com/repos/metoppv/improver/events","assignees_url":"https://api.github.com/repos/metoppv/improver/assignees{/user}","branches_url":"https://api.github.com/repos/metoppv/improver/branches{/branch}","tags_url":"https://api.github.com/repos/metoppv/improver/tags","blobs_url":"https://api.github.com/repos/metoppv/improver/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/metoppv/improver/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/metoppv/improver/git/refs{/sha}","trees_url":"https://api.github.com/repos/metoppv/improver/git/trees{/sha}","statuses_url":"https://api.github.com/repos/metoppv/improver/statuses/{sha}","languages_url":"https://api.github.com/repos/metoppv/improver/languages","stargazers_url":"https://api.github.com/repos/metoppv/improver/stargazers","contributors_url":"https://api.github.com/repos/metoppv/improver/contributors","subscribers_url":"https://api.github.com/repos/metoppv/improver/subscribers","subscription_url":"https://api.github.com/repos/metoppv/improver/subscription","commits_url":"https://api.github.com/repos/metoppv/improver/commits{/sha}","git_commits_url":"https://api.github.com/repos/metoppv/improver/git/commits{/sha}","comments_url":"https://api.github.com/repos/metoppv/improver/comments{/number}","issue_comment_url":"https://api.github.com/repos/metoppv/improver/issues/comments{/number}","contents_url":"https://api.github.com/repos/metoppv/improver/contents/{+path}","compare_url":"https://api.github.com/repos/metoppv/improver/compare/{base}...{head}","merges_url":"https://api.github.com/repos/metoppv/improver/merges","archive_url":"https://api.github.com/repos/metoppv/improver/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/metoppv/improver/downloads","issues_url":"https://api.github.com/repos/metoppv/improver/issues{/number}","pulls_url":"https://api.github.com/repos/metoppv/improver/pulls{/number}","milestones_url":"https://api.github.com/repos/metoppv/improver/milestones{/number}","notifications_url":"https://api.github.com/repos/metoppv/improver/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/metoppv/improver/labels{/name}","releases_url":"https://api.github.com/repos/metoppv/improver/releases{/id}","deployments_url":"https://api.github.com/repos/metoppv/improver/deployments","created_at":"2017-03-17T16:51:29Z","updated_at":"2018-04-26T14:07:08Z","pushed_at":"2018-04-26T14:52:32Z","git_url":"git://github.com/metoppv/improver.git","ssh_url":"git@github.com:metoppv/improver.git","clone_url":"https://github.com/metoppv/improver.git","svn_url":"https://github.com/metoppv/improver","homepage":"http://improver.readthedocs.io/en/latest/","size":3247,"stargazers_count":14,"watchers_count":14,"language":"Python","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":27,"mirror_url":null,"archived":false,"open_issues_count":120,"license":{"key":"other","name":"Other","spdx_id":null,"url":null},"forks":27,"open_issues":120,"watchers":14,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/metoppv/improver/pulls/520"},"html":{"href":"https://github.com/metoppv/improver/pull/520"},"issue":{"href":"https://api.github.com/repos/metoppv/improver/issues/520"},"comments":{"href":"https://api.github.com/repos/metoppv/improver/issues/520/comments"},"review_comments":{"href":"https://api.github.com/repos/metoppv/improver/pulls/520/comments"},"review_comment":{"href":"https://api.github.com/repos/metoppv/improver/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/metoppv/improver/pulls/520/commits"},"statuses":{"href":"https://api.github.com/repos/metoppv/improver/statuses/e4c3fc3410a79f5af87ed1f23a449c1bf62f4b82"}},"author_association":"CONTRIBUTOR"}}
{"actor":{"display_login":"arh89"}}