Repository: niconoe/pyinaturalist
Pull request #151 (2021-05-28 17:47:07)

Use POST /observation_photos to upload photos for create_observation() and update_observation()
Fixes issue #150 

Noticed this note in the [API docs](https://www.inaturalist.org/pages/api+reference#post-observations):
> **local_photos[]**
    List of fields containing uploaded photo data. Request must have a Content-Type of "multipart." We recommend that you use the POST /observation_photos endpoint instead. 


Patch:
diff --git a/HISTORY.md b/HISTORY.md
index 4152652..7c54232 100644
--- a/HISTORY.md
+++ b/HISTORY.md
@@ -8,6 +8,7 @@
     * All can still be imported via `from pyinaturalist import *`
     * Imports from `pyinaturalist.rest_api` and `pyinaturalist.node_api` will still work, but are
       deprecated and will be removed in a future release
+* Update `create_observation()` and `update_observation()` to use `POST /observation_photos` endpoint to upload photos
 
 -----
 ## 0.13 (2021-05-22)
diff --git a/pyinaturalist/request_params.py b/pyinaturalist/request_params.py
index f53546c..475381a 100644
--- a/pyinaturalist/request_params.py
+++ b/pyinaturalist/request_params.py
@@ -316,11 +316,6 @@ def ensure_file_obj(photo: FileOrPath) -> BinaryIO:
     return photo
 
 
-def ensure_file_objs(photos: Iterable[FileOrPath]) -> Iterable[BinaryIO]:
-    """Given one or more file objects and/or paths, read any paths into a file-like object"""
-    return [ensure_file_obj(photo) for photo in ensure_list(photos)]
-
-
 def ensure_list(values: Any):
     """If the value is a string or comma-separated list of values, convert it into a list"""
     if not values:
diff --git a/pyinaturalist/v0/observation_fields.py b/pyinaturalist/v0/observation_fields.py
index c968252..8d64dfb 100644
--- a/pyinaturalist/v0/observation_fields.py
+++ b/pyinaturalist/v0/observation_fields.py
@@ -83,7 +83,7 @@ def put_observation_field_values(
         observation_id: ID of the observation receiving this observation field value
         observation_field_id: ID of the observation field for this observation field value
         value: Value for the observation field
-        access_token: access_token: The access token, as returned by :func:`get_access_token()`
+        access_token: The access token, as returned by :func:`get_access_token()`
         user_agent: A user-agent string that will be passed to iNaturalist.
 
     Returns:
diff --git a/pyinaturalist/v0/observations.py b/pyinaturalist/v0/observations.py
index 1f845c2..231e889 100644
--- a/pyinaturalist/v0/observations.py
+++ b/pyinaturalist/v0/observations.py
@@ -1,3 +1,4 @@
+from logging import getLogger
 from typing import List, Union
 
 from pyinaturalist import api_docs as docs
@@ -11,11 +12,13 @@ from pyinaturalist.request_params import (
     REST_OBS_ORDER_BY_PROPERTIES,
     convert_observation_fields,
     ensure_file_obj,
-    ensure_file_objs,
+    ensure_list,
     validate_multiple_choice_param,
 )
 from pyinaturalist.response_format import convert_all_coordinates, convert_all_timestamps
 
+logger = getLogger(__name__)
+
 
 @document_request_params(
     [
@@ -97,7 +100,7 @@ def get_observations(**params) -> Union[List, str]:
 
 
 @document_request_params([docs._access_token, docs._create_observation])
-def create_observation(access_token: str = None, **params) -> ListResponse:
+def create_observation(access_token: str, **params) -> ListResponse:
     """Create a new observation.
 
     **API reference:** https://www.inaturalist.org/pages/api+reference#post-observations
@@ -137,8 +140,9 @@ def create_observation(access_token: str = None, **params) -> ListResponse:
     if 'observation' in params:
         params.update(params.pop('observation'))
     params = convert_observation_fields(params)
-    if 'local_photos' in params:
-        params['local_photos'] = ensure_file_objs(params['local_photos'])
+
+    # Split out photos to upload separately
+    photos = ensure_list(params.pop('local_photos', None))
 
     response = post(
         url=f'{API_V0_BASE_URL}/observations.json',
@@ -146,7 +150,12 @@ def create_observation(access_token: str = None, **params) -> ListResponse:
         access_token=access_token,
     )
     response.raise_for_status()
-    return response.json()
+    response_json = response.json()
+    observation_id = response_json[0]['id']
+
+    for photo in photos:
+        add_photo_to_observation(observation_id, photo=photo, access_token=access_token)
+    return response_json
 
 
 @document_request_params(
@@ -159,7 +168,7 @@ def create_observation(access_token: str = None, **params) -> ListResponse:
 )
 def update_observation(
     observation_id: int,
-    access_token: str = None,
+    access_token: str,
     **params,
 ) -> ListResponse:
     """
@@ -197,15 +206,15 @@ def update_observation(
             error 410 if the observation doesn't exists or belongs to another user.
     """
     # Accept either top-level params (like most other endpoints)
-    # or nested params (like the iNat API actually accepts)
+    # or nested params (like the API accepts)
     if 'observation' in params:
         params.update(params.pop('observation'))
     params = convert_observation_fields(params)
-    if 'local_photos' in params:
-        params['local_photos'] = ensure_file_objs(params['local_photos'])
 
-    # This is the one Boolean parameter that's specified as an int, for some reason.
-    # Also, set it to True if not specified, which seems like much saner default behavior.
+    # Split out photos to upload separately
+    photos = ensure_list(params.pop('local_photos', None))
+
+    # Note: this is the only Boolean parameter that's specified as an int
     if 'ignore_photos' in params:
         params['ignore_photos'] = int(params['ignore_photos'])
     else:
@@ -217,6 +226,9 @@ def update_observation(
         access_token=access_token,
     )
     response.raise_for_status()
+
+    for photo in photos:
+        add_photo_to_observation(observation_id, photo=photo, access_token=access_token)
     return response.json()
 
 


Test patch:
diff --git a/scripts/observation_crud_test.py b/scripts/observation_crud_test.py
index 7a99f4a..a80c54b 100644
--- a/scripts/observation_crud_test.py
+++ b/scripts/observation_crud_test.py
@@ -1,4 +1,5 @@
 #!/usr/bin/env python
+# flake8: noqa: F401
 """A partially automated script for testing all observation CRUD endpoints,
 in absence of integration tests.
 iNat credentials must be provided via environment variables.
@@ -14,6 +15,7 @@ python scripts/obs_crud_test.py
 ```
 """
 from datetime import datetime
+from logging import basicConfig
 from os.path import join
 from pprint import pprint
 
@@ -23,13 +25,13 @@ from pyinaturalist import (
     delete_observation,
     get_access_token,
     get_observation,
+    put_observation_field_values,
     update_observation,
 )
 from pyinaturalist.constants import SAMPLE_DATA_DIR
 
-# put_observation_field_values,
-
 SAMPLE_PHOTO = join(SAMPLE_DATA_DIR, 'obs_image.jpg')
+basicConfig(level='INFO')
 
 
 def run_observation_crud_test():
@@ -46,7 +48,10 @@ def create_test_obs(token):
     response = create_observation(
         taxon_id=54327,
         observed_on_string=datetime.now().isoformat(),
-        description='This is a test observation used by pyinaturalist, and will be deleted shortly.',
+        description=(
+            'This is a test observation used for testing [pyinaturalist](https://github.com/niconoe/pyinaturalist), '
+            'and will be deleted shortly.'
+        ),
         tag_list='wasp, Belgium',
         latitude=50.647143,
         longitude=4.360216,
@@ -54,6 +59,7 @@ def create_test_obs(token):
         geoprivacy='open',
         access_token=token,
         observation_fields={297: 1},
+        local_photos=[SAMPLE_PHOTO, SAMPLE_PHOTO],
     )
     test_obs_id = response[0]['id']
     print(f'Created new observation: {test_obs_id}')
@@ -101,7 +107,6 @@ def delete_test_obs(test_obs_id, token):
     response = delete_observation(test_obs_id, token)
     # Empty response is expected
     print('Deleted observation')
-    pprint(response, indent=2)
 
 
 if __name__ == '__main__':
diff --git a/test/conftest.py b/test/conftest.py
index 7cc0349..5dafd58 100644
--- a/test/conftest.py
+++ b/test/conftest.py
@@ -5,10 +5,11 @@ Pytest will also automatically pick up any fixtures defined here.
 import json
 import logging
 import os
+import pytest
 import re
 from inspect import Parameter, getmembers, isfunction, signature
 from os.path import join
-from unittest.mock import MagicMock
+from unittest.mock import MagicMock, patch
 
 # If ipdb is installed, register it as the default debugger
 from pyinaturalist.constants import SAMPLE_DATA_DIR
@@ -41,6 +42,13 @@ logging.basicConfig(level='INFO')
 logging.getLogger('pyinaturalist').setLevel('DEBUG')
 
 
+@pytest.fixture(scope='function', autouse=True)
+def patch_ratelimit():
+    """Disable rate-limiting during test session"""
+    with patch('pyinaturalist.api_requests.ratelimit'):
+        yield
+
+
 def get_module_functions(module):
     """Get all functions belonging to a module (excluding imports)"""
     return {
diff --git a/test/v0/test_observations_v0.py b/test/v0/test_observations_v0.py
index ca87f3b..8ac492c 100644
--- a/test/v0/test_observations_v0.py
+++ b/test/v0/test_observations_v0.py
@@ -54,6 +54,48 @@ def test_get_observations__invalid_format(response_format):
         get_observations(taxon_id=493595, response_format=response_format)
 
 
+def test_create_observation(requests_mock):
+    requests_mock.post(
+        f'{API_V0_BASE_URL}/observations.json',
+        json=load_sample_data('create_observation_result.json'),
+        status_code=200,
+    )
+
+    response = create_observation(species_guess='Pieris rapae', access_token='valid token')
+    assert len(response) == 1
+    assert response[0]['latitude'] is None
+    assert response[0]['taxon_id'] == 55626
+
+
+@patch('pyinaturalist.v0.observations.add_photo_to_observation')
+@patch('pyinaturalist.v0.observations.post')
+def test_create_observation__with_photos(post, mock_add_photo):
+    create_observation(access_token='token', local_photos=['photo_1.jpg', 'photo_2.jpg'])
+
+    assert 'local_photos' not in post.call_args[1]['json']['observation']
+    assert mock_add_photo.call_count == 2
+
+
+def test_create_observation_fail(requests_mock):
+    params = {
+        'species_guess': 'Pieris rapae',
+        # Some invalid data so the observation is rejected...
+        'observed_on_string': (datetime.now() + timedelta(days=1)).isoformat(),
+        'latitude': 200,
+    }
+
+    requests_mock.post(
+        f'{API_V0_BASE_URL}/observations.json',
+        json=load_sample_data('create_observation_fail.json'),
+        status_code=422,
+    )
+
+    with pytest.raises(HTTPError) as excinfo:
+        create_observation(access_token='valid token', **params)
+    assert excinfo.value.response.status_code == 422
+    assert 'errors' in excinfo.value.response.json()  # iNat also give details about the errors
+
+
 def test_update_observation(requests_mock):
     requests_mock.put(
         f'{API_V0_BASE_URL}/observations/17932425.json',
@@ -73,14 +115,13 @@ def test_update_observation(requests_mock):
     assert response[0]['description'] == 'updated description v2 !'
 
 
-@patch('pyinaturalist.v0.observations.ensure_file_objs')
+@patch('pyinaturalist.v0.observations.add_photo_to_observation')
 @patch('pyinaturalist.v0.observations.put')
-def test_update_observation__local_photo(put, ensure_file_objs):
+def test_update_observation__local_photo(put, mock_add_photo):
     update_observation(1234, access_token='token', local_photos='photo.jpg')
 
-    # Make sure local_photos is replaced with the output of ensure_file_objs
-    called_params = put.call_args[1]['json']['observation']
-    assert called_params['local_photos'] == ensure_file_objs.return_value
+    assert 'local_photos' not in put.call_args[1]['json']['observation']
+    assert mock_add_photo.call_count == 1
 
 
 def test_update_nonexistent_observation(requests_mock):
@@ -123,49 +164,6 @@ def test_update_observation_not_mine(requests_mock):
     assert excinfo.value.response.json() == {'error': 'Cette observation n’existe plus.'}
 
 
-def test_create_observation(requests_mock):
-    requests_mock.post(
-        f'{API_V0_BASE_URL}/observations.json',
-        json=load_sample_data('create_observation_result.json'),
-        status_code=200,
-    )
-
-    response = create_observation(species_guess='Pieris rapae', access_token='valid token')
-    assert len(response) == 1  # We added a single one
-    assert response[0]['latitude'] is None
-    assert response[0]['taxon_id'] == 55626  # Pieris Rapae @ iNaturalist
-
-
-@patch('pyinaturalist.v0.observations.ensure_file_objs')
-@patch('pyinaturalist.v0.observations.post')
-def test_create_observation__local_photo(post, ensure_file_objs):
-    create_observation(access_token='token', local_photos='photo.jpg')
-
-    # Make sure local_photos is replaced with the output of ensure_file_objs
-    called_params = post.call_args[1]['json']['observation']
-    assert called_params['local_photos'] == ensure_file_objs.return_value
-
-
-def test_create_observation_fail(requests_mock):
-    params = {
-        'species_guess': 'Pieris rapae',
-        # Some invalid data so the observation is rejected...
-        'observed_on_string': (datetime.now() + timedelta(days=1)).isoformat(),
-        'latitude': 200,
-    }
-
-    requests_mock.post(
-        f'{API_V0_BASE_URL}/observations.json',
-        json=load_sample_data('create_observation_fail.json'),
-        status_code=422,
-    )
-
-    with pytest.raises(HTTPError) as excinfo:
-        create_observation(access_token='valid token', **params)
-    assert excinfo.value.response.status_code == 422
-    assert 'errors' in excinfo.value.response.json()  # iNat also give details about the errors
-
-
 def test_add_photo_to_observation(requests_mock):
     requests_mock.post(
         f'{API_V0_BASE_URL}/observation_photos',