From eb10afeaa77f71024e31c645bb03058ce825315c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ayta=C3=A7=20PA=C3=87AL?=
 <25981867+aytacpacal@users.noreply.github.com>
Date: Tue, 12 Jan 2021 00:47:06 +0100
Subject: [PATCH 01/21] Subregion selection

---
 era5cli/cli.py   | 20 ++++++++++++++++++--
 era5cli/fetch.py |  7 ++++++-
 era5cli/utils.py |  1 +
 3 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/era5cli/cli.py b/era5cli/cli.py
index a231604..a465c46 100644
--- a/era5cli/cli.py
+++ b/era5cli/cli.py
@@ -144,6 +144,20 @@ def _build_parser():
                              ''')
     )
 
+    rgn = argparse.ArgumentParser(add_help=False)
+
+    rgn.add_argument(
+        "--area", nargs='+',
+        required=False, type=float,
+        default=[90, -180, -90, 180],
+        help=textwrap.dedent('''
+                            Coordinates to download data for. Defaults to whole
+                            available region. For subregion extraction, provide coordinates 
+                            as ymax,xmin,ymin,xmax
+
+                            ''')
+    )
+
     mnth = argparse.ArgumentParser(add_help=False)
 
     mnth.add_argument(
@@ -187,7 +201,7 @@ def _build_parser():
     )
 
     hourly = subparsers.add_parser(
-        'hourly', parents=[common, mnth, day, hour],
+        'hourly', parents=[common, rgn, mnth, day, hour],
         description='Execute the data fetch process for hourly data.',
         prog=textwrap.dedent('''\
                              Use "era5cli hourly --help" for more information.
@@ -211,7 +225,7 @@ def _build_parser():
     )
 
     monthly = subparsers.add_parser(
-        'monthly', parents=[common, mnth],
+        'monthly', parents=[common, mnth, rgn],
         description='Execute the data fetch process for monthly data.',
         prog=textwrap.dedent('''\
                              Use "era5cli monthly --help" for more information.
@@ -238,6 +252,7 @@ def _build_parser():
                              ''')
     )
 
+
     info = subparsers.add_parser(
         'info',
         description='Show information on available variables and levels.',
@@ -362,6 +377,7 @@ def _execute(args):
             days=days,
             hours=hours,
             variables=args.variables,
+            area=args.area,
             outputformat=args.format,
             outputprefix=args.outputprefix,
             period=args.command,
diff --git a/era5cli/fetch.py b/era5cli/fetch.py
index d67b1e6..c366f9a 100644
--- a/era5cli/fetch.py
+++ b/era5cli/fetch.py
@@ -26,6 +26,8 @@ class Fetch:
             hours to download data for.
         variables: list(str)
             List of variable names to download data for.
+        area: list(int)
+            Lon-lat values to download data for. (-180 and 180, -90 and 90)
         outputformat: str
             Type of file to download: 'netcdf' or 'grib'.
         outputprefix: str
@@ -62,7 +64,7 @@ class Fetch:
     """
 
     def __init__(self, years: list, months: list, days: list,
-                 hours: list, variables: list, outputformat: str,
+                 hours: list, variables: list, area: list, outputformat: str,
                  outputprefix: str, period: str, ensemble: bool,
                  statistics=None, synoptic=None, pressurelevels=None,
                  merge=False, threads=None, prelimbe=False):
@@ -84,6 +86,8 @@ def __init__(self, years: list, months: list, days: list,
         """list(int): List of pressure levels."""
         self.variables = variables
         """list(str): List of variables."""
+        self.area = area
+        """list(int): Lİst of coordinates"""
         self.outputformat = outputformat
         """str: File format of output file."""
         self.years = years
@@ -230,6 +234,7 @@ def _build_request(self, variable, years):
                    'product_type': self._product_type(),
                    'month': self.months,
                    'time': self.hours,
+                   'area': self.area,
                    'format': self.outputformat}
 
         # variable is pressure level variable
diff --git a/era5cli/utils.py b/era5cli/utils.py
index 3442791..0da82b5 100644
--- a/era5cli/utils.py
+++ b/era5cli/utils.py
@@ -187,3 +187,4 @@ def _append_netcdf_history(ncfile: str, appendtxt: str):
     except AttributeError:
         ncfile.history = appendtxt
     ncfile.close()
+

From 352404f871460a724c2830354382b2d1a5ec4feb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ayta=C3=A7=20PA=C3=87AL?=
 <25981867+aytacpacal@users.noreply.github.com>
Date: Tue, 12 Jan 2021 01:18:55 +0100
Subject: [PATCH 02/21] Modified README

---
 README.rst       | 1 +
 era5cli/cli.py   | 2 +-
 era5cli/fetch.py | 2 +-
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/README.rst b/README.rst
index 3078fdd..cacf3ca 100644
--- a/README.rst
+++ b/README.rst
@@ -33,6 +33,7 @@ With era5cli you can: ​
 - select multiple variables for several months and years
 - split outputs by years, producing a separate file for every year instead of merging them in one file
 - download multiple files at once
+- download subregion
 - download ERA5 back extension (preliminary version)
 
 .. inclusion-marker-end-do-not-remove
diff --git a/era5cli/cli.py b/era5cli/cli.py
index a465c46..7237dc6 100644
--- a/era5cli/cli.py
+++ b/era5cli/cli.py
@@ -153,7 +153,7 @@ def _build_parser():
         help=textwrap.dedent('''
                             Coordinates to download data for. Defaults to whole
                             available region. For subregion extraction, provide coordinates 
-                            as ymax,xmin,ymin,xmax
+                            as {ymax,xmin,ymin,xmax}
 
                             ''')
     )
diff --git a/era5cli/fetch.py b/era5cli/fetch.py
index c366f9a..d0bdd01 100644
--- a/era5cli/fetch.py
+++ b/era5cli/fetch.py
@@ -26,7 +26,7 @@ class Fetch:
             hours to download data for.
         variables: list(str)
             List of variable names to download data for.
-        area: list(int)
+        area: list(float)
             Lon-lat values to download data for. (-180 and 180, -90 and 90)
         outputformat: str
             Type of file to download: 'netcdf' or 'grib'.

From 5ef80697caf36c6b72c4a7d287a2dbc66afa8fd9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ayta=C3=A7=20PA=C3=87AL?=
 <25981867+aytacpacal@users.noreply.github.com>
Date: Tue, 12 Jan 2021 01:22:16 +0100
Subject: [PATCH 03/21] Modified README

---
 era5cli/cli.py   | 3 +--
 era5cli/fetch.py | 2 +-
 era5cli/utils.py | 3 +--
 3 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/era5cli/cli.py b/era5cli/cli.py
index 7237dc6..1c35fce 100644
--- a/era5cli/cli.py
+++ b/era5cli/cli.py
@@ -225,7 +225,7 @@ def _build_parser():
     )
 
     monthly = subparsers.add_parser(
-        'monthly', parents=[common, mnth, rgn],
+        'monthly', parents=[common, rgn, mnth],
         description='Execute the data fetch process for monthly data.',
         prog=textwrap.dedent('''\
                              Use "era5cli monthly --help" for more information.
@@ -252,7 +252,6 @@ def _build_parser():
                              ''')
     )
 
-
     info = subparsers.add_parser(
         'info',
         description='Show information on available variables and levels.',
diff --git a/era5cli/fetch.py b/era5cli/fetch.py
index d0bdd01..7d1b0b1 100644
--- a/era5cli/fetch.py
+++ b/era5cli/fetch.py
@@ -87,7 +87,7 @@ def __init__(self, years: list, months: list, days: list,
         self.variables = variables
         """list(str): List of variables."""
         self.area = area
-        """list(int): Lİst of coordinates"""
+        """list(int): List of coordinates"""
         self.outputformat = outputformat
         """str: File format of output file."""
         self.years = years
diff --git a/era5cli/utils.py b/era5cli/utils.py
index 0da82b5..ce06ccc 100644
--- a/era5cli/utils.py
+++ b/era5cli/utils.py
@@ -186,5 +186,4 @@ def _append_netcdf_history(ncfile: str, appendtxt: str):
             {}'''.format(appendtxt, ncfile.history))
     except AttributeError:
         ncfile.history = appendtxt
-    ncfile.close()
-
+    ncfile.close()
\ No newline at end of file

From 08bd892cd0d646ca51b3ed9c643f5dfc939cd3cc Mon Sep 17 00:00:00 2001
From: Barbara Vreede <b.vreede@esciencecenter.nl>
Date: Thu, 18 Feb 2021 17:49:17 +0100
Subject: [PATCH 04/21] Apply PR66 review corrections to parse area coordinates

---
 era5cli/cli.py      | 22 +++++++++++-----------
 era5cli/fetch.py    | 13 ++++++++-----
 era5cli/utils.py    |  2 +-
 tests/test_fetch.py |  4 +++-
 4 files changed, 23 insertions(+), 18 deletions(-)

diff --git a/era5cli/cli.py b/era5cli/cli.py
index 2bf72f9..d67d19f 100644
--- a/era5cli/cli.py
+++ b/era5cli/cli.py
@@ -159,16 +159,16 @@ def _build_parser():
                              ''')
     )
 
-    rgn = argparse.ArgumentParser(add_help=False)
-
-    rgn.add_argument(
-        "--area", nargs='+',
-        required=False, type=float,
-        default=[90, -180, -90, 180],
+    common.add_argument(
+        "--area", nargs=4, type=float,
+        required=False,
         help=textwrap.dedent('''
-                            Coordinates to download data for. Defaults to whole
-                            available region. For subregion extraction, provide coordinates 
-                            as {ymax,xmin,ymin,xmax}
+                            Use this argument to specify a subregion for which
+                            to download the data. Coordinates should be given
+                            as a space separated list with ymax xmin ymin xmax
+                            (N W S E); e.g. --area 90 -180 -90 180.
+                            Without specification, the entire available area
+                            will be returned.
 
                             ''')
     )
@@ -216,7 +216,7 @@ def _build_parser():
     )
 
     hourly = subparsers.add_parser(
-        'hourly', parents=[common, rgn, mnth, day, hour],
+        'hourly', parents=[common, mnth, day, hour],
         description='Execute the data fetch process for hourly data.',
         prog=textwrap.dedent('''\
                              Use "era5cli hourly --help" for more information.
@@ -240,7 +240,7 @@ def _build_parser():
     )
 
     monthly = subparsers.add_parser(
-        'monthly', parents=[common, rgn, mnth],
+        'monthly', parents=[common, mnth],
         description='Execute the data fetch process for monthly data.',
         prog=textwrap.dedent('''\
                              Use "era5cli monthly --help" for more information.
diff --git a/era5cli/fetch.py b/era5cli/fetch.py
index 4f50d66..44aa41c 100644
--- a/era5cli/fetch.py
+++ b/era5cli/fetch.py
@@ -26,8 +26,10 @@ class Fetch:
             hours to download data for.
         variables: list(str)
             List of variable names to download data for.
-        area: list(float)
-            Lon-lat values to download data for. (-180 and 180, -90 and 90)
+        area: None, list(float)
+            Coordinates in case extraction of a subregion is requested.
+            Specified as [N, W, S, E] or [ymax, xmin, ymin, xmax], with x and y
+            in the range -180, +180 and -90, +90, respectively.
         outputformat: str
             Type of file to download: 'netcdf' or 'grib'.
         outputprefix: str
@@ -66,8 +68,8 @@ class Fetch:
     """
 
     def __init__(self, years: list, months: list, days: list,
-                 hours: list, variables: list, area: list, outputformat: str,
-                 outputprefix: str, period: str, ensemble: bool,
+                 hours: list, variables: list, outputformat: str,
+                 outputprefix: str, period: str, ensemble: bool, area=None,
                  statistics=None, synoptic=None, pressurelevels=None,
                  merge=False, threads=None, prelimbe=False, land=False):
         """Initialization of Fetch class."""
@@ -90,7 +92,8 @@ def __init__(self, years: list, months: list, days: list,
         self.variables = variables
         """list(str): List of variables."""
         self.area = area
-        """list(int): List of coordinates"""
+        """list(float): Coordinates specifying the subregion that will be
+        extracted. Default is None for whole available area."""
         self.outputformat = outputformat
         """str: File format of output file."""
         self.years = years
diff --git a/era5cli/utils.py b/era5cli/utils.py
index ce06ccc..3442791 100644
--- a/era5cli/utils.py
+++ b/era5cli/utils.py
@@ -186,4 +186,4 @@ def _append_netcdf_history(ncfile: str, appendtxt: str):
             {}'''.format(appendtxt, ncfile.history))
     except AttributeError:
         ncfile.history = appendtxt
-    ncfile.close()
\ No newline at end of file
+    ncfile.close()
diff --git a/tests/test_fetch.py b/tests/test_fetch.py
index e51516c..3fac571 100644
--- a/tests/test_fetch.py
+++ b/tests/test_fetch.py
@@ -7,7 +7,8 @@
 
 def initialize(outputformat='netcdf', merge=False, statistics=None,
                synoptic=None, ensemble=True, pressurelevels=None,
-               threads=2, period='hourly', variables=['total_precipitation'],
+               threads=2, period='hourly', area=None,
+               variables=['total_precipitation'],
                years=[2008, 2009], months=list(range(1, 13)),
                days=list(range(1, 32)), hours=list(range(0, 24)),
                prelimbe=False, land=False):
@@ -16,6 +17,7 @@ def initialize(outputformat='netcdf', merge=False, statistics=None,
                        months=months,
                        days=days,
                        hours=hours,
+                       area=area,
                        variables=variables,
                        outputformat=outputformat,
                        outputprefix='era5',

From a5db94f3e6f817254693e1b8005f3f83f99c84bf Mon Sep 17 00:00:00 2001
From: Barbara Vreede <b.vreede@esciencecenter.nl>
Date: Thu, 18 Feb 2021 17:50:31 +0100
Subject: [PATCH 05/21] parse and verify area coordinates

---
 era5cli/fetch.py    | 28 +++++++++++++++++++++++++
 tests/test_fetch.py | 50 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 78 insertions(+)

diff --git a/era5cli/fetch.py b/era5cli/fetch.py
index 44aa41c..b86566c 100644
--- a/era5cli/fetch.py
+++ b/era5cli/fetch.py
@@ -275,6 +275,31 @@ def _check_variable(self, variable):
                 "Invalid variable name: {}".format(variable)
             )
 
+    def _check_area(self):
+        """Confirm that area parameters are correct."""
+        (N, W, S, E) = self.area
+        # breakpoint()
+        if not (-90 <= N <= 90
+                and -90 <= S <= 90
+                and -180 <= W <= 180
+                and -180 <= E <= 180
+                and N > S
+                and W != E
+                ):
+            raise ValueError(
+                "Provide coordinates as N W S E, or ymax xmin ymin xmax. "
+                "x must be in range -180,+180 and y must be in range -90,+90."
+            )
+
+    def _parse_area(self):
+        """Parse area parameters to accepted coordinates."""
+        self._check_area()
+        area = [round(coord, ndigits=2) for coord in self.area]
+        if self.area != area:
+            print(
+                f"NB: coordinates {self.area} rounded down to two decimals.\n")
+        return(area)
+
     def _build_name(self, variable):
         """Build up name of dataset to use"""
 
@@ -318,6 +343,9 @@ def _build_request(self, variable, years):
             self._check_levels()
             request["pressure_level"] = self.pressure_levels
 
+        if self.area:
+            request["area"] = self._parse_area()
+
         product_type = self._product_type()
         if product_type is not None:
             request["product_type"] = product_type
diff --git a/tests/test_fetch.py b/tests/test_fetch.py
index 3fac571..0c017b9 100644
--- a/tests/test_fetch.py
+++ b/tests/test_fetch.py
@@ -520,3 +520,53 @@ def test_more_incompatible_options():
     era5 = initialize(statistics=True, ensemble=False)
     with pytest.raises(ValueError):
         era5._build_request('total_precipitation', [2008])
+
+
+def test_area():
+    '''Test that area is parsed properly'''
+    era5 = initialize()
+    assert era5.area is None
+
+    era5 = initialize(area=[90, -180, -90, 180])
+    (name, request) = era5._build_request('total_precipitation', [2008])
+    assert era5.area == [90, -180, -90, 180]
+    assert request["area"] == [90, -180, -90, 180]
+
+    # Decimals are rounded down
+    era5 = initialize(area=[89.9999, -179.90, -90.0000, 179.012])
+    (name, request) = era5._build_request('total_precipitation', [2008])
+    assert request["area"] == [90.0, -179.90, -90.0, 179.01]
+
+    # North lower than South
+    with pytest.raises(ValueError):
+        era5 = initialize(area=[-10, -180, 10, 180])
+        era5._build_request('total_precipitation', [2008])
+
+    # North equals South
+    with pytest.raises(ValueError):
+        era5 = initialize(area=[0, -180, 0, 180])
+        era5._build_request('total_precipitation', [2008])
+
+    # East equals West
+    with pytest.raises(ValueError):
+        era5 = initialize(area=[90, 0, -90, 0])
+        era5._build_request('total_precipitation', [2008])
+
+    # North, West, South, East out of bounds
+    with pytest.raises(ValueError):
+        era5 = initialize(area=[1000, -180, -90, 180])
+        era5._build_request('total_precipitation', [2008])
+    with pytest.raises(ValueError):
+        era5 = initialize(area=[90, 1000, -90, 180])
+        era5._build_request('total_precipitation', [2008])
+    with pytest.raises(ValueError):
+        era5 = initialize(area=[90, -180, 1000, 180])
+        era5._build_request('total_precipitation', [2008])
+    with pytest.raises(ValueError):
+        era5 = initialize(area=[90, -180, -90, 1000])
+        era5._build_request('total_precipitation', [2008])
+
+    # Coordinate missing
+    with pytest.raises(ValueError):
+        era5 = initialize(area=[-180, 180, -90])
+        era5._build_request('total_precipitation', [2008])

From a4f99cafb2b321989abb1fc0a4426576f34d9a37 Mon Sep 17 00:00:00 2001
From: Barbara Vreede <b.vreede@esciencecenter.nl>
Date: Thu, 18 Feb 2021 17:58:56 +0100
Subject: [PATCH 06/21] Test cli input for area coordinates

---
 tests/test_cli.py | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/tests/test_cli.py b/tests/test_cli.py
index 0906589..4ca77ac 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -27,6 +27,52 @@ def test_parse_args():
     assert args.statistics
     assert not args.threads
     assert args.variables == ['total_precipitation']
+    assert not args.area
+
+
+def test_area_values():
+    '''Test if correct area values are expected'''
+    # Test if area arguments are parsed correctly
+    argv = ['hourly', '--startyear', '2008',
+            '--variables', 'total_precipitation', '--statistics',
+            '--endyear', '2008', '--ensemble',
+            '--area', '90', '-180', '-90', '180']
+    args = cli._parse_args(argv)
+    assert args.area == [90, -180, -90, 180]
+
+    # missing the argument defaults to area None
+    argv = ['hourly', '--startyear', '2008',
+            '--variables', 'total_precipitation', '--statistics',
+            '--endyear', '2008', '--ensemble']
+    args = cli._parse_args(argv)
+    assert not args.area
+
+
+@pytest.mark.xfail(reason='I am not sure how to call the right error message')
+def test_area_values_xfail():
+    # Requires four values
+    with pytest.raises(ArgumentError):
+        argv = ['hourly', '--startyear', '2008',
+                '--variables', 'total_precipitation', '--statistics',
+                '--endyear', '2008', '--ensemble',
+                '--area', '90', '-180', '-90']
+        cli._parse_args(argv)
+
+    # A value cannot be missing
+    with pytest.raises(ArgumentError):
+        argv = ['hourly', '--startyear', '2008',
+                '--variables', 'total_precipitation', '--statistics',
+                '--endyear', '2008', '--ensemble',
+                '--area', '90', '-180', '-90', '']
+        cli._parse_args(argv)
+
+    # Values must be numeric
+    with pytest.raises(ArgumentError):
+        argv = ['hourly', '--startyear', '2008',
+                '--variables', 'total_precipitation', '--statistics',
+                '--endyear', '2008', '--ensemble',
+                '--area', '90', '-180', '-90', 'E']
+        cli._parse_args(argv)
 
 
 def test_period_args():

From e07db17ad30c3b36ea183316426db3025e6165a0 Mon Sep 17 00:00:00 2001
From: Barbara Vreede <b.vreede@esciencecenter.nl>
Date: Fri, 19 Feb 2021 06:14:00 +0100
Subject: [PATCH 07/21] Match cli and fetch documentation

---
 era5cli/cli.py   | 14 ++++++++------
 era5cli/fetch.py |  4 +++-
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/era5cli/cli.py b/era5cli/cli.py
index d67d19f..4e37c30 100644
--- a/era5cli/cli.py
+++ b/era5cli/cli.py
@@ -162,12 +162,14 @@ def _build_parser():
     common.add_argument(
         "--area", nargs=4, type=float,
         required=False,
-        help=textwrap.dedent('''
-                            Use this argument to specify a subregion for which
-                            to download the data. Coordinates should be given
-                            as a space separated list with ymax xmin ymin xmax
-                            (N W S E); e.g. --area 90 -180 -90 180.
-                            Without specification, the entire available area
+        help=textwrap.dedent('''\
+                            Coordinates in case extraction of a subregion is
+                            requested. Specified as N W S E or
+                            ymax xmin ymin xmax with x and y in the
+                            range -180, +180 and -90, +90, respectively
+                            e.g. --area 90 -180 -90 180.
+                            Requests are rounded down to two decimals.
+                            Without specification, the whole available area
                             will be returned.
 
                             ''')
diff --git a/era5cli/fetch.py b/era5cli/fetch.py
index b86566c..323fdcf 100644
--- a/era5cli/fetch.py
+++ b/era5cli/fetch.py
@@ -29,7 +29,9 @@ class Fetch:
         area: None, list(float)
             Coordinates in case extraction of a subregion is requested.
             Specified as [N, W, S, E] or [ymax, xmin, ymin, xmax], with x and y
-            in the range -180, +180 and -90, +90, respectively.
+            in the range -180, +180 and -90, +90, respectively. Requests are
+            rounded down to two decimals. Without specification, the whole
+            available area will be returned.
         outputformat: str
             Type of file to download: 'netcdf' or 'grib'.
         outputprefix: str

From f017b65b4aa7800e6c43386eed1f265d0288e15f Mon Sep 17 00:00:00 2001
From: Barbara Vreede <b.vreede@esciencecenter.nl>
Date: Fri, 19 Feb 2021 08:29:31 +0100
Subject: [PATCH 08/21] add area to filename

---
 era5cli/fetch.py | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/era5cli/fetch.py b/era5cli/fetch.py
index 323fdcf..d015ff1 100644
--- a/era5cli/fetch.py
+++ b/era5cli/fetch.py
@@ -167,6 +167,10 @@ def _define_outputfilename(self, var, years):
                   else self.outputprefix)
         yearblock = f"{start}-{end}" if not start == end else f"{start}"
         fname = f"{prefix}_{var}_{yearblock}_{self.period}"
+        if self.area:
+            directions = ['N', 'W', 'S', 'E']
+            coords = [str(c) for c in self._round_area()]
+            fname += '_' + ''.join([d+c for d, c in zip(directions, coords)])
         if self.ensemble:
             fname += "_ensemble"
         if self.statistics:
@@ -293,14 +297,18 @@ def _check_area(self):
                 "x must be in range -180,+180 and y must be in range -90,+90."
             )
 
+    def _round_area(self):
+        area = [round(coord, ndigits=2) for coord in self.area]
+        return area
+
     def _parse_area(self):
         """Parse area parameters to accepted coordinates."""
         self._check_area()
-        area = [round(coord, ndigits=2) for coord in self.area]
+        area = self._round_area()
         if self.area != area:
             print(
                 f"NB: coordinates {self.area} rounded down to two decimals.\n")
-        return(area)
+        return area
 
     def _build_name(self, variable):
         """Build up name of dataset to use"""

From 37018e22ef5b4e155aeea4656fcec905fc30555d Mon Sep 17 00:00:00 2001
From: Barbara Vreede <b.vreede@esciencecenter.nl>
Date: Fri, 19 Feb 2021 08:30:53 +0100
Subject: [PATCH 09/21] change error to satisfy flake8

---
 tests/test_cli.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/test_cli.py b/tests/test_cli.py
index 4ca77ac..d5e7028 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -51,7 +51,7 @@ def test_area_values():
 @pytest.mark.xfail(reason='I am not sure how to call the right error message')
 def test_area_values_xfail():
     # Requires four values
-    with pytest.raises(ArgumentError):
+    with pytest.raises(ValueError):
         argv = ['hourly', '--startyear', '2008',
                 '--variables', 'total_precipitation', '--statistics',
                 '--endyear', '2008', '--ensemble',
@@ -59,7 +59,7 @@ def test_area_values_xfail():
         cli._parse_args(argv)
 
     # A value cannot be missing
-    with pytest.raises(ArgumentError):
+    with pytest.raises(ValueError):
         argv = ['hourly', '--startyear', '2008',
                 '--variables', 'total_precipitation', '--statistics',
                 '--endyear', '2008', '--ensemble',
@@ -67,7 +67,7 @@ def test_area_values_xfail():
         cli._parse_args(argv)
 
     # Values must be numeric
-    with pytest.raises(ArgumentError):
+    with pytest.raises(ValueError):
         argv = ['hourly', '--startyear', '2008',
                 '--variables', 'total_precipitation', '--statistics',
                 '--endyear', '2008', '--ensemble',

From 57024b8d0ae786c7ff1adad653f5dd83956f63e6 Mon Sep 17 00:00:00 2001
From: Barbara Vreede <b.vreede@esciencecenter.nl>
Date: Fri, 19 Feb 2021 08:59:56 +0100
Subject: [PATCH 10/21] add test for area coords in outputfilename

---
 tests/test_fetch.py | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/tests/test_fetch.py b/tests/test_fetch.py
index 0c017b9..1fc5da3 100644
--- a/tests/test_fetch.py
+++ b/tests/test_fetch.py
@@ -202,6 +202,20 @@ def test_define_outputfilename():
     fn = 'era5-land_total_precipitation_2008_hourly.nc'
     assert fname == fn
 
+    era5.area = [90.0, -180.0, -90.0, 180.0]
+    fname = era5._define_outputfilename('total_precipitation', [2008])
+    fn = (
+        'era5-land_total_precipitation_2008_hourly_N90.0W-180.0S-90.0E180.0.nc'
+    )
+    assert fname == fn
+
+    era5.area = [90.0, -180.0, -80.999, 170.001]
+    fname = era5._define_outputfilename('total_precipitation', [2008])
+    fn = (
+        'era5-land_total_precipitation_2008_hourly_N90.0W-180.0S-81.0E170.0.nc'
+    )
+    assert fname == fn
+
 
 def test_number_outputfiles(capsys):
     """Test function for the number of outputs."""

From 496959bebcdca29e297620cb3f5d51a17a767773 Mon Sep 17 00:00:00 2001
From: Barbara Vreede <b.vreede@esciencecenter.nl>
Date: Fri, 19 Feb 2021 10:06:54 +0100
Subject: [PATCH 11/21] Error is a system
 exithttps://stackoverflow.com/questions/14728376/i-want-python-argparse-to-throw-an-exception-rather-than-usage

---
 tests/test_cli.py | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/tests/test_cli.py b/tests/test_cli.py
index d5e7028..0ce1f5d 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -47,11 +47,8 @@ def test_area_values():
     args = cli._parse_args(argv)
     assert not args.area
 
-
-@pytest.mark.xfail(reason='I am not sure how to call the right error message')
-def test_area_values_xfail():
     # Requires four values
-    with pytest.raises(ValueError):
+    with pytest.raises(SystemExit):
         argv = ['hourly', '--startyear', '2008',
                 '--variables', 'total_precipitation', '--statistics',
                 '--endyear', '2008', '--ensemble',
@@ -59,7 +56,7 @@ def test_area_values_xfail():
         cli._parse_args(argv)
 
     # A value cannot be missing
-    with pytest.raises(ValueError):
+    with pytest.raises(SystemExit):
         argv = ['hourly', '--startyear', '2008',
                 '--variables', 'total_precipitation', '--statistics',
                 '--endyear', '2008', '--ensemble',
@@ -67,7 +64,7 @@ def test_area_values_xfail():
         cli._parse_args(argv)
 
     # Values must be numeric
-    with pytest.raises(ValueError):
+    with pytest.raises(SystemExit):
         argv = ['hourly', '--startyear', '2008',
                 '--variables', 'total_precipitation', '--statistics',
                 '--endyear', '2008', '--ensemble',

From 64897cae3e137b991efde0d438628b501dbf9d59 Mon Sep 17 00:00:00 2001
From: Barbara Vreede <b.vreede@esciencecenter.nl>
Date: Fri, 19 Feb 2021 10:08:01 +0100
Subject: [PATCH 12/21] Update era5cli/fetch.py

Co-authored-by: Peter Kalverla <peter.kalverla@gmx.com>
---
 era5cli/fetch.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/era5cli/fetch.py b/era5cli/fetch.py
index d015ff1..f018861 100644
--- a/era5cli/fetch.py
+++ b/era5cli/fetch.py
@@ -284,7 +284,6 @@ def _check_variable(self, variable):
     def _check_area(self):
         """Confirm that area parameters are correct."""
         (N, W, S, E) = self.area
-        # breakpoint()
         if not (-90 <= N <= 90
                 and -90 <= S <= 90
                 and -180 <= W <= 180

From eee0211a482a27fc4460e088afdb489a42eaf5d0 Mon Sep 17 00:00:00 2001
From: Barbara Vreede <b.vreede@esciencecenter.nl>
Date: Fri, 19 Feb 2021 18:15:21 +0100
Subject: [PATCH 13/21] adjust readme description

---
 README.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/README.rst b/README.rst
index cacf3ca..e0a0c30 100644
--- a/README.rst
+++ b/README.rst
@@ -33,10 +33,10 @@ With era5cli you can: ​
 - select multiple variables for several months and years
 - split outputs by years, producing a separate file for every year instead of merging them in one file
 - download multiple files at once
-- download subregion
+- extract data for a sub-region of the globe
 - download ERA5 back extension (preliminary version)
 
 .. inclusion-marker-end-do-not-remove
 
-| Free software: Apache Software License 2.0 
+| Free software: Apache Software License 2.0
 | Documentation: https://era5cli.readthedocs.io

From b6d2e459e074a4432f135e21016477b04586d933 Mon Sep 17 00:00:00 2001
From: Barbara Vreede <b.vreede@esciencecenter.nl>
Date: Tue, 2 Mar 2021 12:42:36 +0100
Subject: [PATCH 14/21] remove coordinate confusion and use xmax xmin ymax ymin
 consistently

---
 era5cli/cli.py      |  2 +-
 era5cli/fetch.py    | 29 ++++++++++++-----------------
 tests/test_fetch.py | 17 +++++++++++------
 3 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/era5cli/cli.py b/era5cli/cli.py
index 4e37c30..6991a97 100644
--- a/era5cli/cli.py
+++ b/era5cli/cli.py
@@ -164,7 +164,7 @@ def _build_parser():
         required=False,
         help=textwrap.dedent('''\
                             Coordinates in case extraction of a subregion is
-                            requested. Specified as N W S E or
+                            requested. Specified as
                             ymax xmin ymin xmax with x and y in the
                             range -180, +180 and -90, +90, respectively
                             e.g. --area 90 -180 -90 180.
diff --git a/era5cli/fetch.py b/era5cli/fetch.py
index 39c6890..5d2d069 100644
--- a/era5cli/fetch.py
+++ b/era5cli/fetch.py
@@ -28,7 +28,7 @@ class Fetch:
             List of variable names to download data for.
         area: None, list(float)
             Coordinates in case extraction of a subregion is requested.
-            Specified as [N, W, S, E] or [ymax, xmin, ymin, xmax], with x and y
+            Specified as [ymax, xmin, ymin, xmax], with x and y
             in the range -180, +180 and -90, +90, respectively. Requests are
             rounded down to two decimals. Without specification, the whole
             available area will be returned.
@@ -167,9 +167,8 @@ def _define_outputfilename(self, var, years):
         yearblock = f"{start}-{end}" if not start == end else f"{start}"
         fname = f"{prefix}_{var}_{yearblock}_{self.period}"
         if self.area:
-            directions = ['N', 'W', 'S', 'E']
-            coords = [str(c) for c in self._round_area()]
-            fname += '_' + ''.join([d+c for d, c in zip(directions, coords)])
+            coords = [str(int(c)) for c in self.area]
+            fname += '_[' + ']['.join(coords) + ']'
         if self.ensemble:
             fname += "_ensemble"
         if self.statistics:
@@ -282,27 +281,23 @@ def _check_variable(self, variable):
 
     def _check_area(self):
         """Confirm that area parameters are correct."""
-        (N, W, S, E) = self.area
-        if not (-90 <= N <= 90
-                and -90 <= S <= 90
-                and -180 <= W <= 180
-                and -180 <= E <= 180
-                and N > S
-                and W != E
+        (ymax, xmin, ymin, xmax) = self.area
+        if not (-90 <= ymax <= 90
+                and -90 <= ymin <= 90
+                and -180 <= xmin <= 180
+                and -180 <= xmax <= 180
+                and ymax > ymin
+                and xmax != xmin
                 ):
             raise ValueError(
-                "Provide coordinates as N W S E, or ymax xmin ymin xmax. "
+                "Provide coordinates as ymax xmin ymin xmax. "
                 "x must be in range -180,+180 and y must be in range -90,+90."
             )
 
-    def _round_area(self):
-        area = [round(coord, ndigits=2) for coord in self.area]
-        return area
-
     def _parse_area(self):
         """Parse area parameters to accepted coordinates."""
         self._check_area()
-        area = self._round_area()
+        area = [round(coord, ndigits=2) for coord in self.area]
         if self.area != area:
             print(
                 f"NB: coordinates {self.area} rounded down to two decimals.\n")
diff --git a/tests/test_fetch.py b/tests/test_fetch.py
index 5313c19..e8ec28e 100644
--- a/tests/test_fetch.py
+++ b/tests/test_fetch.py
@@ -206,14 +206,14 @@ def test_define_outputfilename():
     era5.area = [90.0, -180.0, -90.0, 180.0]
     fname = era5._define_outputfilename('total_precipitation', [2008])
     fn = (
-        'era5-land_total_precipitation_2008_hourly_N90.0W-180.0S-90.0E180.0.nc'
+        'era5-land_total_precipitation_2008_hourly_[90][-180][-90][180].nc'
     )
     assert fname == fn
 
     era5.area = [90.0, -180.0, -80.999, 170.001]
     fname = era5._define_outputfilename('total_precipitation', [2008])
     fn = (
-        'era5-land_total_precipitation_2008_hourly_N90.0W-180.0S-81.0E170.0.nc'
+        'era5-land_total_precipitation_2008_hourly_[90][-180][-80][170].nc'
     )
     assert fname == fn
 
@@ -553,22 +553,27 @@ def test_area():
     (name, request) = era5._build_request('total_precipitation', [2008])
     assert request["area"] == [90.0, -179.90, -90.0, 179.01]
 
-    # North lower than South
+    # ymax may not be lower than ymin
     with pytest.raises(ValueError):
         era5 = initialize(area=[-10, -180, 10, 180])
         era5._build_request('total_precipitation', [2008])
 
-    # North equals South
+    # xmin higher than xmax should be ok
+    era5 = initialize(area=[90, 120, -90, -120])
+    (name, request) = era5._build_request('total_precipitation', [2008])
+    assert request["area"] == [90.0, 120.0, -90.0, -120.0]
+
+    # ymax may not equal ymin
     with pytest.raises(ValueError):
         era5 = initialize(area=[0, -180, 0, 180])
         era5._build_request('total_precipitation', [2008])
 
-    # East equals West
+    # xmin may not equal xmax
     with pytest.raises(ValueError):
         era5 = initialize(area=[90, 0, -90, 0])
         era5._build_request('total_precipitation', [2008])
 
-    # North, West, South, East out of bounds
+    # ymax, xmin, ymin, xmax may not be out of bounds
     with pytest.raises(ValueError):
         era5 = initialize(area=[1000, -180, -90, 180])
         era5._build_request('total_precipitation', [2008])

From 5119c24dcebcaa851e1e11aec6b5e6db6506656d Mon Sep 17 00:00:00 2001
From: Barbara Vreede <b.vreede@esciencecenter.nl>
Date: Tue, 2 Mar 2021 12:46:02 +0100
Subject: [PATCH 15/21] Update tests/test_fetch.py

Co-authored-by: Peter Kalverla <peter.kalverla@gmx.com>
---
 tests/test_fetch.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/test_fetch.py b/tests/test_fetch.py
index e8ec28e..af9bbb1 100644
--- a/tests/test_fetch.py
+++ b/tests/test_fetch.py
@@ -539,7 +539,7 @@ def test_more_incompatible_options():
 
 
 def test_area():
-    '''Test that area is parsed properly'''
+    """Test that area is parsed properly."""
     era5 = initialize()
     assert era5.area is None
 

From 4d3d0cacaa03a5944d70cfa1e430691a584adbd7 Mon Sep 17 00:00:00 2001
From: Barbara Vreede <b.vreede@esciencecenter.nl>
Date: Tue, 2 Mar 2021 12:46:37 +0100
Subject: [PATCH 16/21] Update tests/test_cli.py

Co-authored-by: Peter Kalverla <peter.kalverla@gmx.com>
---
 tests/test_cli.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/test_cli.py b/tests/test_cli.py
index 7f51725..4c83a4f 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -32,7 +32,7 @@ def test_parse_args():
 
 
 def test_area_values():
-    '''Test if correct area values are expected'''
+    """Test if area argument is parsed correctly."""
     # Test if area arguments are parsed correctly
     argv = ['hourly', '--startyear', '2008',
             '--variables', 'total_precipitation', '--statistics',

From 7b2afb40e8f6b789d90711199a530f6bf8265a6b Mon Sep 17 00:00:00 2001
From: Barbara Vreede <b.vreede@esciencecenter.nl>
Date: Tue, 2 Mar 2021 12:47:00 +0100
Subject: [PATCH 17/21] Update tests/test_cli.py

Co-authored-by: Peter Kalverla <peter.kalverla@gmx.com>
---
 tests/test_cli.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/test_cli.py b/tests/test_cli.py
index 4c83a4f..241fa25 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -41,7 +41,7 @@ def test_area_values():
     args = cli._parse_args(argv)
     assert args.area == [90, -180, -90, 180]
 
-    # missing the argument defaults to area None
+    # Check that area defaults to None
     argv = ['hourly', '--startyear', '2008',
             '--variables', 'total_precipitation', '--statistics',
             '--endyear', '2008', '--ensemble']

From dce63fb36b7a633f35ccd0b6d1563f766fe3924d Mon Sep 17 00:00:00 2001
From: Barbara Vreede <b.vreede@esciencecenter.nl>
Date: Tue, 2 Mar 2021 12:51:58 +0100
Subject: [PATCH 18/21] Update tests/test_cli.py

Co-authored-by: Peter Kalverla <peter.kalverla@gmx.com>
---
 tests/test_cli.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/test_cli.py b/tests/test_cli.py
index 241fa25..5b0fc84 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -31,7 +31,7 @@ def test_parse_args():
     assert not args.area
 
 
-def test_area_values():
+def test_area_argument():
     """Test if area argument is parsed correctly."""
     # Test if area arguments are parsed correctly
     argv = ['hourly', '--startyear', '2008',

From 87ddf8e329603d48b42a41d0f02693d67f217691 Mon Sep 17 00:00:00 2001
From: Barbara Vreede <b.vreede@esciencecenter.nl>
Date: Thu, 4 Mar 2021 07:15:38 +0100
Subject: [PATCH 19/21] changed output filename to include coordinates with
 NSEW notation

---
 era5cli/fetch.py    | 10 ++++++++--
 tests/test_fetch.py | 11 +++++++++--
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/era5cli/fetch.py b/era5cli/fetch.py
index 5d2d069..938b44b 100644
--- a/era5cli/fetch.py
+++ b/era5cli/fetch.py
@@ -158,6 +158,13 @@ def _extension(self):
             raise ValueError('Unknown outputformat: {}'.format(
                 self.outputformat))
 
+    def _process_areaname(self):
+        (ymax,xmin,ymin,xmax) = [round(c) for c in self.area]
+        lon = lambda x: f"{x}E" if x>0 else(f"{abs(x)}W" if x<0 else 0)
+        lat = lambda y:  f"{y}N" if y>0 else(f"{abs(y)}S" if y<0 else 0)
+        name = f"_{lon(xmin)}-{lon(xmax)}_{lat(ymin)}-{lat(ymax)}"
+        return name
+
     def _define_outputfilename(self, var, years):
         """Define output filename."""
         start, end = years[0], years[-1]
@@ -167,8 +174,7 @@ def _define_outputfilename(self, var, years):
         yearblock = f"{start}-{end}" if not start == end else f"{start}"
         fname = f"{prefix}_{var}_{yearblock}_{self.period}"
         if self.area:
-            coords = [str(int(c)) for c in self.area]
-            fname += '_[' + ']['.join(coords) + ']'
+            fname += self._process_areaname()
         if self.ensemble:
             fname += "_ensemble"
         if self.statistics:
diff --git a/tests/test_fetch.py b/tests/test_fetch.py
index af9bbb1..b7d1924 100644
--- a/tests/test_fetch.py
+++ b/tests/test_fetch.py
@@ -206,14 +206,21 @@ def test_define_outputfilename():
     era5.area = [90.0, -180.0, -90.0, 180.0]
     fname = era5._define_outputfilename('total_precipitation', [2008])
     fn = (
-        'era5-land_total_precipitation_2008_hourly_[90][-180][-90][180].nc'
+        'era5-land_total_precipitation_2008_hourly_180W-180E_90S-90N.nc'
     )
     assert fname == fn
 
     era5.area = [90.0, -180.0, -80.999, 170.001]
     fname = era5._define_outputfilename('total_precipitation', [2008])
     fn = (
-        'era5-land_total_precipitation_2008_hourly_[90][-180][-80][170].nc'
+        'era5-land_total_precipitation_2008_hourly_180W-170E_81S-90N.nc'
+    )
+    assert fname == fn
+
+    era5.area = [0, 120, -90, 180]
+    fname = era5._define_outputfilename('total_precipitation', [2008])
+    fn = (
+        'era5-land_total_precipitation_2008_hourly_120E-180E_90S-0.nc'
     )
     assert fname == fn
 

From c961d1c48aeeb2094d36d61225b50dcbe9deac4e Mon Sep 17 00:00:00 2001
From: Barbara Vreede <b.vreede@esciencecenter.nl>
Date: Thu, 4 Mar 2021 07:21:30 +0100
Subject: [PATCH 20/21] fix spaces for flake8

---
 era5cli/fetch.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/era5cli/fetch.py b/era5cli/fetch.py
index 938b44b..3dbc22b 100644
--- a/era5cli/fetch.py
+++ b/era5cli/fetch.py
@@ -159,9 +159,9 @@ def _extension(self):
                 self.outputformat))
 
     def _process_areaname(self):
-        (ymax,xmin,ymin,xmax) = [round(c) for c in self.area]
-        lon = lambda x: f"{x}E" if x>0 else(f"{abs(x)}W" if x<0 else 0)
-        lat = lambda y:  f"{y}N" if y>0 else(f"{abs(y)}S" if y<0 else 0)
+        (ymax, xmin, ymin, xmax) = [round(c) for c in self.area]
+        def lon(x): return f"{x}E" if x > 0 else(f"{abs(x)}W" if x < 0 else 0)
+        def lat(y): return f"{y}N" if y > 0 else(f"{abs(y)}S" if y < 0 else 0)
         name = f"_{lon(xmin)}-{lon(xmax)}_{lat(ymin)}-{lat(ymax)}"
         return name
 

From aec813afd849fbeb7ed77dee13a0c5f9e42f799c Mon Sep 17 00:00:00 2001
From: Barbara Vreede <b.vreede@esciencecenter.nl>
Date: Thu, 4 Mar 2021 10:47:47 +0100
Subject: [PATCH 21/21] add N/E to 0 coordinate in name

---
 era5cli/fetch.py    | 4 ++--
 tests/test_fetch.py | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/era5cli/fetch.py b/era5cli/fetch.py
index 3dbc22b..2ff8fc8 100644
--- a/era5cli/fetch.py
+++ b/era5cli/fetch.py
@@ -160,8 +160,8 @@ def _extension(self):
 
     def _process_areaname(self):
         (ymax, xmin, ymin, xmax) = [round(c) for c in self.area]
-        def lon(x): return f"{x}E" if x > 0 else(f"{abs(x)}W" if x < 0 else 0)
-        def lat(y): return f"{y}N" if y > 0 else(f"{abs(y)}S" if y < 0 else 0)
+        def lon(x): return f"{x}E" if x >= 0 else f"{abs(x)}W"
+        def lat(y): return f"{y}N" if y >= 0 else f"{abs(y)}S"
         name = f"_{lon(xmin)}-{lon(xmax)}_{lat(ymin)}-{lat(ymax)}"
         return name
 
diff --git a/tests/test_fetch.py b/tests/test_fetch.py
index b7d1924..701801e 100644
--- a/tests/test_fetch.py
+++ b/tests/test_fetch.py
@@ -220,7 +220,7 @@ def test_define_outputfilename():
     era5.area = [0, 120, -90, 180]
     fname = era5._define_outputfilename('total_precipitation', [2008])
     fn = (
-        'era5-land_total_precipitation_2008_hourly_120E-180E_90S-0.nc'
+        'era5-land_total_precipitation_2008_hourly_120E-180E_90S-0N.nc'
     )
     assert fname == fn