Repository: ESMValGroup/ESMValCore

Issue:
extract_region forces the region to be represented from 0-360 longitude
`extract_region` uses an iris cube `intersection`, and then applies another so that the cube is represented from 0-360 longitude. If the extracted region crosses 360 degrees, this forces the bounds to be discontiguous, and prevents future regrid-ing. I see that, in the default preprocessing order, regrid-ing is done before extracting a region. However I can see no obvious benefit to forcing the cube to be stored from 0-360 degrees, and it complicates analysis down the line outside of the preprocessor. I propose that this line be removed.


Discussion:
rebeccaherman1: Since I have not done pull requests before, it's a lot to take in and I do not fully understand what I'm looking at or how to figure it out regarding the unit tests etc. Guidance and feedback appreciated! (@valeriupredoi? :) )
valeriupredoi: @rebeccaherman1 welcome to the developers' bunch of ESMValCore! Cheers for the PR, let me know when you are happy with it (bar tests and other more advanced features) and I can come by here do a review and try help with the tests etc :+1: :beer: 
rebeccaherman1: @valeriupredoi I do just want to remove that one line which I think is causing me problems with no obvious gain.
It is not a new feature, but rather a small adjustment to an existing feature, so I do not feel I need to add any unit tests or documentation.

The problem is that I do not know why that line was originally included -- there may have been a reason motivated by some future function which may have to be modified to respond to this -- I don't know. Or maybe it just seemed like a good idea at the time and is actually not important. 

I did not understand how to run tests on my own, so I tried just submitting the PR to see what would happen with the existing unit tests. But, of course, it is now failing one, and I don't understand how to figure out what test it is failing and why.
valeriupredoi: @zklaus would you, by any remote chance. have some time to run a few recipes with this, during your release tests, plsss? @schlunma (and myself, recently converted) believes this should work fine to avoid the non-contiguity issue that Rebecca raise in the linked issue :beer: 
bouweandela: > Am I meant to fill out the checklist below? Still trying to understand the instructions for pull requests.

Yes. If you click the links you will find more information that explains what the checkbox is all about. A general introduction to contributing to ESMValCore is available here: https://docs.esmvaltool.org/projects/ESMValCore/en/latest/contributing.html
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/ESMValGroup/ESMValCore/pull/2230?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ESMValGroup) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Comparison is base [(`444cdbf`)](https://app.codecov.io/gh/ESMValGroup/ESMValCore/commit/444cdbf46876d3fd1d6bc86cac318d72983230e1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ESMValGroup) 93.47% compared to head [(`a03cfaa`)](https://app.codecov.io/gh/ESMValGroup/ESMValCore/pull/2230?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ESMValGroup) 93.47%.
> Report is 24 commits behind head on main.


<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main    #2230      +/-   ##
==========================================
- Coverage   93.47%   93.47%   -0.01%     
==========================================
  Files         238      238              
  Lines       12932    12931       -1     
==========================================
- Hits        12088    12087       -1     
  Misses        844      844              
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/ESMValGroup/ESMValCore/pull/2230?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ESMValGroup).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ESMValGroup).

rebeccaherman1: @bouweandela Thanks for the tip about running tests in the linked thread. 
It turns out that the one test that was failing was a test that targets the functionality I wished to change. 
@valeriupredoi Now that I've modified the expected output in that one tests, all tests pass. Would you be willing to do the review?
Are we still hoping for @zklaus to test some recipes?
I have not checked the "backwards compatible" checkbox above because this is a change in specs, but a good one.

valeriupredoi: @rebeccaherman1 good work! Shall do, in a jiffy :+1:
bouweandela: Could you please check if other preprocessor functions rely on the behavior that is getting removed here?
rebeccaherman1: @bouweandela see no sign that any other preprocessor relies on this behavior.
bouweandela: Thanks for checking!

Patch:
diff --git a/esmvalcore/preprocessor/_area.py b/esmvalcore/preprocessor/_area.py
index ae78526a2..145b87ec3 100644
--- a/esmvalcore/preprocessor/_area.py
+++ b/esmvalcore/preprocessor/_area.py
@@ -84,7 +84,6 @@ def extract_region(
             latitude=(start_latitude, end_latitude),
             ignore_bounds=True,
         )
-        region_subset = region_subset.intersection(longitude=(0., 360.))
     else:
         region_subset = _extract_irregular_region(
             cube,


Test patch:
diff --git a/tests/unit/preprocessor/_area/test_area.py b/tests/unit/preprocessor/_area/test_area.py
index a74ac8ba0..3939437f3 100644
--- a/tests/unit/preprocessor/_area/test_area.py
+++ b/tests/unit/preprocessor/_area/test_area.py
@@ -770,7 +770,7 @@ def test_crop_cube_with_ne_file(ne_ocean_shapefile):
         result = _crop_cube(cube, *geometries.bounds, cmor_coords=False)
         result = (result.coord("latitude").points[-1],
                   result.coord("longitude").points[-1])
-        expected = (89., 359.)
+        expected = (89., 179.)
         np.testing.assert_allclose(result, expected)