Issue #280 simplify BatchJob methods (start, stop, describe., ...)

Keep legacy aliases (without triggering warnings for now)
Introduce decorator for openEO endpoint cross-referencing

Author: soxofaan

CHANGELOG.md

@@ -13,6 +13,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- Simplified `BatchJob` methods `start()`, `stop()`, `describe()`, ...
+  Legacy aliases `start_job()`, `describe_job()`, ... are still available and don't trigger a deprecation warning for now.
+  ([#280](https://github.com/Open-EO/openeo-python-client/issues/280))
+
 ### Removed
 
 ### Fixed

docs/batch_jobs.rst

@@ -168,11 +168,11 @@ Run a batch job
 =================
 
 Starting a batch job is pretty straightforward with the
-:py:meth:`~openeo.rest.job.BatchJob.start_job()` method:
+:py:meth:`~openeo.rest.job.BatchJob.start()` method:
 
 .. code-block:: python
 
-    job.start_job()
+    job.start()
 
 If this didn't raise any errors or exceptions your job
 should now have started (status "running")

openeo/extra/job_management.py

@@ -336,7 +336,7 @@ def _launch_job(self, start_job, df, i, backend_name):
                     if status == "created":
                         # start job if not yet done by callback
                         try:
-                            job.start_job()
+                            job.start()
                             df.loc[i, "status"] = job.status()
                         except OpenEoApiError as e:
                             _log.error(e)
@@ -355,7 +355,7 @@ def on_job_done(self, job: BatchJob, row):
         """
         # TODO: param `row` is never accessed in this method. Remove it? Is this intended for future use?
 
-        job_metadata = job.describe_job()
+        job_metadata = job.describe()
         job_dir = self.get_job_dir(job.job_id)
         metadata_path = self.get_job_metadata_path(job.job_id)
 
@@ -415,7 +415,7 @@ def _update_statuses(self, df: pd.DataFrame):
             try:
                 con = self._get_connection(backend_name)
                 the_job = con.job(job_id)
-                job_metadata = the_job.describe_job()
+                job_metadata = the_job.describe()
                 _log.info(
                     f"Status of job {job_id!r} (on backend {backend_name}) is {job_metadata['status']!r}"
                 )

openeo/internal/documentation.py

@@ -2,15 +2,16 @@
 Utilities to build/automate/extend documentation
 """
 import collections
+import inspect
 import textwrap
 from functools import partial
-from typing import Callable, Optional
+from typing import Callable, Optional, Tuple
 
 # TODO: give this a proper public API?
 _process_registry = collections.defaultdict(list)
 
 
-def openeo_process(f: Callable = None, process_id: Optional[str] = None, mode: Optional[str] = None):
+def openeo_process(f: Optional[Callable] = None, process_id: Optional[str] = None, mode: Optional[str] = None):
     """
     Decorator for function or method to associate it with a standard openEO process
 
@@ -34,3 +35,22 @@ def openeo_process(f: Callable = None, process_id: Optional[str] = None, mode: O
 
     _process_registry[process_id].append((f, mode))
     return f
+
+
+def openeo_endpoint(endpoint: str) -> Callable[[Callable], Callable]:
+    """
+    Parameterized decorator to annotate given function or method with the openEO endpoint it interacts with
+
+    :param endpoint: REST endpoint (e.g. "GET /jobs", "POST /result", ...)
+    :return:
+    """
+    # TODO: automatically parse/normalize endpoint (to method+path)
+    # TODO: wrap this in some markup/directive to make this more a "small print" note.
+
+    def decorate(f: Callable) -> Callable:
+        is_method = list(inspect.signature(f).parameters.keys())[:1] == ["self"]
+        seealso = f"This {'method' if is_method else 'function'} uses openEO endpoint ``{endpoint}``"
+        f.__doc__ = textwrap.dedent(f.__doc__ or "") + "\n\n" + seealso + "\n"
+        return f
+
+    return decorate

openeo/internal/warnings.py

@@ -34,15 +34,19 @@ def test_warnings(stacklevel=1):
         )
 
 
-def legacy_alias(orig: Callable, name: str, since: str):
+def legacy_alias(orig: Callable, name: str = "n/a", *, since: str, mode: str = "full"):
     """
     Create legacy alias of given function/method/classmethod/staticmethod
 
     :param orig: function/method to create legacy alias for
-    :param name: name of the alias
+    :param name: name of the alias (unused)
     :param since: version since when this is alias is deprecated
+    :param mode:
+        - "full": raise warnings on calling, only have deprecation note as doc
+        - "soft": don't raise warning on calling, just add deprecation note to doc
     :return:
     """
+    # TODO: drop `name` argument?
     post_process = None
     if isinstance(orig, classmethod):
         post_process = classmethod
@@ -64,13 +68,18 @@ def legacy_alias(orig: Callable, name: str, since: str):
     def wrapper(*args, **kwargs):
         return orig(*args, **kwargs)
 
-    # Drop original doc block, just show deprecation note.
-    wrapper.__doc__ = ""
     ref = f":py:{'meth' if 'method' in kind else 'func'}:`.{orig.__name__}`"
-    wrapper = deprecated(
-        reason=f"Use of this legacy {kind} is deprecated, use {ref} instead.",
-        version=since,
-    )(wrapper)
+    message = f"Usage of this legacy {kind} is deprecated. Use {ref} instead."
+
+    if mode == "full":
+        # Drop original doc block, just show deprecation note.
+        wrapper.__doc__ = ""
+        wrapper = deprecated(reason=message, version=since)(wrapper)
+    elif mode == "soft":
+        # Only keep first paragraph of original doc block
+        wrapper.__doc__ = "\n\n".join(orig.__doc__.split("\n\n")[:1] + [f".. deprecated:: {since}\n   {message}\n"])
+    else:
+        raise ValueError(mode)
 
     if post_process:
         wrapper = post_process(wrapper)

openeo/rest/job.py

@@ -9,8 +9,9 @@
 import requests
 
 from openeo.api.logs import LogEntry, normalize_log_level, log_level_name
+from openeo.internal.documentation import openeo_endpoint
 from openeo.internal.jupyter import render_component, render_error, VisualDict, VisualList
-from openeo.internal.warnings import deprecated
+from openeo.internal.warnings import deprecated, legacy_alias
 from openeo.rest import OpenEoClientException, JobFailedException, OpenEoApiError
 from openeo.util import ensure_dir
 
@@ -44,60 +45,81 @@ def __repr__(self):
         return '<{c} job_id={i!r}>'.format(c=self.__class__.__name__, i=self.job_id)
 
     def _repr_html_(self):
-        data = self.describe_job()
+        data = self.describe()
         currency = self.connection.capabilities().currency()
         return render_component('job', data=data, parameters={'currency': currency})
 
-    def describe_job(self) -> dict:
-        """ Get all job information."""
-        # GET /jobs/{job_id}
-        # TODO: rename to just `describe`? #280
+    @openeo_endpoint("GET /jobs/{job_id}")
+    def describe(self) -> dict:
+        """
+        Get detailed metadata about a submitted batch job
+        (title, process graph, status, progress, ...).
+
+        .. versionadded:: 0.20.0
+            This method was previously called :py:meth:`describe_job`.
+        """
         return self.connection.get(f"/jobs/{self.job_id}", expected_status=200).json()
 
+    describe_job = legacy_alias(describe, since="0.20.0", mode="soft")
+
     def status(self) -> str:
         """
         Get the status of the batch job
 
         :return: batch job status, one of "created", "queued", "running", "canceled", "finished" or "error".
         """
-        return self.describe_job().get("status", "N/A")
-
-    def update_job(self, process_graph=None, output_format=None,
-                   output_parameters=None, title=None, description=None,
-                   plan=None, budget=None, additional=None):
-        """ Update a job."""
-        # PATCH /jobs/{job_id}
-        # TODO: rename to just `update`? #280
-        raise NotImplementedError
-
-    def delete_job(self):
-        """ Delete a job."""
-        # DELETE /jobs/{job_id}
-        # TODO: rename to just `delete`? #280
+        return self.describe().get("status", "N/A")
+
+    @openeo_endpoint("DELETE /jobs/{job_id}")
+    def delete(self):
+        """
+        Delete this batch job.
+
+        .. versionadded:: 0.20.0
+            This method was previously called :py:meth:`delete_job`.
+        """
         self.connection.delete(f"/jobs/{self.job_id}", expected_status=204)
 
-    def estimate_job(self):
+    delete_job = legacy_alias(delete, since="0.20.0", mode="soft")
+
+    @openeo_endpoint("GET /jobs/{job_id}/estimate")
+    def estimate(self):
         """Calculate time/cost estimate for a job."""
-        # GET /jobs/{job_id}/estimate
         data = self.connection.get(
             f"/jobs/{self.job_id}/estimate", expected_status=200
         ).json()
         currency = self.connection.capabilities().currency()
         return VisualDict('job-estimate', data=data, parameters={'currency': currency})
 
-    def start_job(self):
-        """ Start / queue a job for processing."""
-        # POST /jobs/{job_id}/results
-        # TODO: rename to just `start`? #280
-        # TODO: return self, to allow chained calls
+    estimate_job = legacy_alias(estimate, since="0.20.0", mode="soft")
+
+    @openeo_endpoint("POST /jobs/{job_id}/results")
+    def start(self) -> "BatchJob":
+        """
+        Start this batch job.
+
+        :return: Started batch job
+
+        .. versionadded:: 0.20.0
+            This method was previously called :py:meth:`start_job`.
+        """
         self.connection.post(f"/jobs/{self.job_id}/results", expected_status=202)
+        return self
 
-    def stop_job(self):
-        """ Stop / cancel job processing."""
-        # DELETE /jobs/{job_id}/results
-        # TODO: rename to just `stop`? #280
+    start_job = legacy_alias(start, since="0.20.0", mode="soft")
+
+    @openeo_endpoint("DELETE /jobs/{job_id}/results")
+    def stop(self):
+        """
+        Stop this batch job.
+
+        .. versionadded:: 0.20.0
+            This method was previously called :py:meth:`stop_job`.
+        """
         self.connection.delete(f"/jobs/{self.job_id}/results", expected_status=204)
 
+    stop_job = legacy_alias(stop, since="0.20.0", mode="soft")
+
     def get_results_metadata_url(self, *, full: bool = False) -> str:
         """Get results metadata URL"""
         url = f"/jobs/{self.job_id}/results"
@@ -232,7 +254,7 @@ def print_status(msg: str):
 
         # TODO: make `max_poll_interval`, `connection_retry_interval` class constants or instance properties?
         print_status("send 'start'")
-        self.start_job()
+        self.start()
 
         # TODO: also add  `wait` method so you can track a job that already has started explicitly
         #   or just rename this method to `wait` and automatically do start if not started yet?
@@ -254,7 +276,7 @@ def soft_error(message: str):
         while True:
             # TODO: also allow a hard time limit on this infinite poll loop?
             try:
-                job_info = self.describe_job()
+                job_info = self.describe()
             except requests.ConnectionError as e:
                 soft_error("Connection error while polling job status: {e}".format(e=e))
                 continue

tests/internal/test_warnings.py

@@ -30,7 +30,7 @@ def add(x, y):
     assert do_plus.__doc__ == (
         "\n"
         ".. deprecated:: v1.2\n"
-        "   Use of this legacy function is deprecated, use :py:func:`.add`\n"
+        "   Usage of this legacy function is deprecated. Use :py:func:`.add`\n"
         "   instead.\n"
     )
 
@@ -41,7 +41,7 @@ def add(x, y):
         UserDeprecationWarning,
         match=re.escape(
             "Call to deprecated function (or staticmethod) add."
-            " (Use of this legacy function is deprecated, use `.add` instead.)"
+            " (Usage of this legacy function is deprecated. Use `.add` instead.)"
             " -- Deprecated since version v1.2."
         ),
     ):
@@ -61,7 +61,7 @@ def add(self, x, y):
     assert Foo.do_plus.__doc__ == (
         "\n"
         ".. deprecated:: v1.2\n"
-        "   Use of this legacy method is deprecated, use :py:meth:`.add`\n"
+        "   Usage of this legacy method is deprecated. Use :py:meth:`.add`\n"
         "   instead.\n"
     )
 
@@ -72,7 +72,7 @@ def add(self, x, y):
         UserDeprecationWarning,
         match=re.escape(
             "Call to deprecated method add."
-            " (Use of this legacy method is deprecated, use `.add` instead.)"
+            " (Usage of this legacy method is deprecated. Use `.add` instead.)"
             " -- Deprecated since version v1.2."
         ),
     ):
@@ -94,7 +94,7 @@ def add(cls, x, y):
     assert Foo.do_plus.__doc__ == (
         "\n"
         ".. deprecated:: v1.2\n"
-        "   Use of this legacy class method is deprecated, use\n"
+        "   Usage of this legacy class method is deprecated. Use\n"
         "   :py:meth:`.add` instead.\n"
     )
 
@@ -104,7 +104,7 @@ def add(cls, x, y):
     expected_warning = re.escape(
         # Workaround for bug in classmethod detection before Python 3.9 (see https://wrapt.readthedocs.io/en/latest/decorators.html#decorating-class-methods
         f"Call to deprecated {'class method' if sys.version_info >= (3, 9) else 'function (or staticmethod)'} add."
-        " (Use of this legacy class method is deprecated, use `.add` instead.)"
+        " (Usage of this legacy class method is deprecated. Use `.add` instead.)"
         " -- Deprecated since version v1.2."
     )
 
@@ -130,7 +130,7 @@ def add(x, y):
     assert Foo.do_plus.__doc__ == (
         "\n"
         ".. deprecated:: v1.2\n"
-        "   Use of this legacy static method is deprecated, use\n"
+        "   Usage of this legacy static method is deprecated. Use\n"
         "   :py:meth:`.add` instead.\n"
     )
 
@@ -139,7 +139,7 @@ def add(x, y):
 
     expected_warning = re.escape(
         "Call to deprecated function (or staticmethod) add."
-        " (Use of this legacy static method is deprecated, use `.add` instead.)"
+        " (Usage of this legacy static method is deprecated. Use `.add` instead.)"
         " -- Deprecated since version v1.2."
     )
     with pytest.warns(UserDeprecationWarning, match=expected_warning):
@@ -151,6 +151,29 @@ def add(x, y):
     assert res == 5
 
 
+def test_legacy_alias_method_soft(recwarn):
+    class Foo:
+        def add(self, x, y):
+            """Add x and y."""
+            return x + y
+
+        do_plus = legacy_alias(add, since="v1.2", mode="soft")
+
+    assert Foo.add.__doc__ == "Add x and y."
+    assert Foo.do_plus.__doc__ == (
+        "Add x and y.\n"
+        "\n"
+        ".. deprecated:: v1.2\n"
+        "   Usage of this legacy method is deprecated. Use :py:meth:`.add` instead.\n"
+    )
+
+    assert Foo().add(2, 3) == 5
+    assert len(recwarn) == 0
+
+    res = Foo().do_plus(2, 3)
+    assert len(recwarn) == 0
+    assert res == 5
+
 
 def test_deprecated_decorator():
     class Foo:

tests/rest/datacube/test_datacube100.py

@@ -2555,7 +2555,7 @@ def test_legacy_send_job(self, con100, requests_mock):
         """Legacy `DataCube.send_job` alis for `create_job"""
         requests_mock.post(API_URL + "/jobs", json=self._get_handler_post_jobs())
         cube = con100.load_collection("S2")
-        expected_warning = "Call to deprecated method create_job. (Use of this legacy method is deprecated, use `.create_job` instead.) -- Deprecated since version 0.10.0."
+        expected_warning = "Call to deprecated method create_job. (Usage of this legacy method is deprecated. Use `.create_job` instead.) -- Deprecated since version 0.10.0."
         with pytest.warns(UserDeprecationWarning, match=re.escape(expected_warning)):
             job = cube.send_job(out_format="GTiff")
         assert job.job_id == "myj0b1"