diff --git a/Readme.md b/Readme.md index 95dfbb3..d4a89d8 100644 --- a/Readme.md +++ b/Readme.md @@ -118,6 +118,18 @@ grobid_client [OPTIONS] SERVICE | `processCitationList` | Parse citation strings | Text files (one citation per line) | | `processCitationPatentST36` | Process patent citations | XML ST36 format | | `processCitationPatentPDF` | Process patent PDFs | PDF files | +| `referenceAnnotations` | Reference annotations (JSON coordinates) | PDF files | +| `annotatePDF` | Annotated PDF with reference/citation annotations | PDF files | +| `citationPatentAnnotations` | Patent citation annotations (JSON coordinates) | Patent PDF files | + +> [!NOTE] +> The annotation services (`referenceAnnotations`, `annotatePDF`, `citationPatentAnnotations`) do not produce TEI XML. +> `referenceAnnotations` and `citationPatentAnnotations` write JSON coordinate files (`*.references.json`, +> `*.patent-citations.json`), while `annotatePDF` writes an annotated PDF (`*.annotated.pdf`). The `--json` and +> `--markdown` conversion options do not apply to these services. The annotation services only honor +> `--consolidate_citations` (and `--include_raw_citations` for `referenceAnnotations`); if you pass any other +> processing option (e.g. `--consolidate_header`, `--teiCoordinates`) the client logs a warning that it will be ignored. +> See [issue #79](https://github.com/grobidOrg/grobid-client-python/issues/79). #### Common Options @@ -167,6 +179,12 @@ grobid_client --server https://grobid.example.com --input ~/citations.txt proces # Force reprocessing with sentence segmentation and JSON output grobid_client --input ~/docs --force --segment_sentences --json processFulltextDocument + +# Reference annotations as JSON coordinates (writes *.references.json) +grobid_client --input ~/pdfs --output ~/annotations referenceAnnotations + +# Annotated PDF with reference/citation annotations (writes *.annotated.pdf) +grobid_client --input ~/pdfs --output ~/annotated annotatePDF ``` ### Python Library diff --git a/grobid_client/client.py b/grobid_client/client.py index 725f0db..2a0a973 100644 --- a/grobid_client/client.py +++ b/grobid_client/client.py @@ -121,7 +121,13 @@ def call_api( ResultParser or ErrorParser. """ headers = deepcopy(headers) or {} - headers["Accept"] = self.accept_type + # Only the fallback: a caller asking for a specific representation must + # be able to get it. The GROBID annotation services answer with JSON + # coordinates or with an annotated PDF rather than with XML, and + # overwriting their Accept header here sent every request out as + # application/xml no matter what the caller passed. + # See https://github.com/grobidOrg/grobid-client-python/issues/79 + headers.setdefault("Accept", self.accept_type) params = deepcopy(params) or {} data = data or {} files = files or {} diff --git a/grobid_client/grobid_client.py b/grobid_client/grobid_client.py index 8176103..0559f20 100644 --- a/grobid_client/grobid_client.py +++ b/grobid_client/grobid_client.py @@ -47,6 +47,34 @@ class GrobidClient(ApiClient): # See https://github.com/grobidOrg/grobid-client-python/issues/54 CONSOLIDATE_CITATIONS_MIN_TIMEOUT = 120 + # Default output descriptor for TEI-producing services: (Accept header, + # output file suffix, binary output). All the "process*" services return + # TEI XML, which is what they have effectively been asking for all along: + # the "text/plain" this client used to pass never reached the wire, since + # call_api replaced it with its own application/xml. + DEFAULT_SERVICE_OUTPUT: Tuple[str, str, bool] = ("application/xml", ".grobid.tei.xml", False) + + # PDF annotation services return either JSON coordinates or an annotated + # (binary) PDF instead of TEI XML, so they need their own Accept header, + # output suffix and write mode. + # See https://github.com/grobidOrg/grobid-client-python/issues/79 + SERVICE_OUTPUTS: dict = { + "referenceAnnotations": ("application/json", ".references.json", False), + "citationPatentAnnotations": ("application/json", ".patent-citations.json", False), + "annotatePDF": ("application/pdf", ".annotated.pdf", True), + } + + # Client options (by their argument name) actually honored by each service. + # Services absent from this map accept the full set of options, so no + # warning is emitted for them. The annotation services (issue #79) only + # look at a subset, so passing anything else is a no-op server-side and the + # user is warned about it. + SERVICE_SUPPORTED_PARAMS: dict = { + "referenceAnnotations": {"consolidate_citations", "include_raw_citations"}, + "citationPatentAnnotations": {"consolidate_citations"}, + "annotatePDF": {"consolidate_citations"}, + } + # Default configuration values DEFAULT_CONFIG: dict = { 'grobid_server': 'http://localhost:8070', @@ -141,6 +169,35 @@ def _warn_on_consolidation_timeout(self, consolidate_citations: bool) -> None: f"(2-3 minutes is recommended)." ) + def _service_output(self, service: str) -> Tuple[str, str, bool]: + """Return the (Accept header, output suffix, is_binary) tuple for a service. + + Annotation services (referenceAnnotations, annotatePDF, + citationPatentAnnotations) produce JSON or binary PDF output; every + other service produces TEI XML. + See https://github.com/grobidOrg/grobid-client-python/issues/79 + """ + return self.SERVICE_OUTPUTS.get(service, self.DEFAULT_SERVICE_OUTPUT) + + def _warn_unsupported_service_params(self, service: str, requested_params: dict) -> None: + """Warn about requested options that the selected service ignores. + + ``requested_params`` maps an option's argument name to whether the user + enabled it. Options that are set but not supported by ``service`` are + silently dropped by GROBID, so we surface them as a warning. + See https://github.com/grobidOrg/grobid-client-python/issues/79 + """ + supported = self.SERVICE_SUPPORTED_PARAMS.get(service) + if supported is None: + return + + ignored = sorted(name for name, is_set in requested_params.items() if is_set and name not in supported) + if ignored: + self.logger.warning( + f"The following option(s) are not supported by the '{service}' service " + f"and will be ignored: {', '.join(ignored)}." + ) + def _handle_server_busy_retry(self, file_path: str, retry_func: Any, *args: Any, **kwargs: Any) -> Any: """Handle server busy (503) retry logic.""" self.logger.warning(f"Server busy (503), retrying {file_path} after {self.config['sleep_time']} seconds") @@ -336,6 +393,7 @@ def _output_file_name( input_file: str, input_path: str, output: Optional[str], + suffix: str = ".grobid.tei.xml", ) -> str: # Use pathlib for consistent cross-platform path handling input_file_path = pathlib.Path(input_file) @@ -344,10 +402,10 @@ def _output_file_name( # Calculate relative path from input_path, then join with output directory input_path_abs = pathlib.Path(input_path).resolve() input_file_rel = input_file_path.resolve().relative_to(input_path_abs) - filename = pathlib.Path(output) / f"{input_file_rel.stem}.grobid.tei.xml" + filename = pathlib.Path(output) / f"{input_file_rel.stem}{suffix}" else: # Use the same directory as the input file - filename = input_file_path.parent / f"{input_file_path.stem}.grobid.tei.xml" + filename = input_file_path.parent / f"{input_file_path.stem}{suffix}" return str(filename) @@ -387,6 +445,19 @@ def process( # See https://github.com/grobidOrg/grobid-client-python/issues/54 self._warn_on_consolidation_timeout(consolidate_citations) + # Warn once if the caller enabled options that this service ignores + # (e.g. consolidate_header on an annotation service). See issue #79. + self._warn_unsupported_service_params(service, { + "generate_ids": generate_ids, + "consolidate_header": consolidate_header, + "consolidate_citations": consolidate_citations, + "include_raw_citations": include_raw_citations, + "include_raw_affiliations": include_raw_affiliations, + "tei_coordinates": tei_coordinates, + "segment_sentences": segment_sentences, + "flavor": bool(flavor), + }) + # First pass: count all eligible files all_input_files = [] for (dirpath, dirnames, filenames) in os.walk(input_path): @@ -518,20 +589,26 @@ def process_batch( error_count = 0 skipped_count = 0 + # Determine the output format for this service. Annotation services + # produce JSON or a binary PDF instead of TEI XML. + _, output_suffix, binary_output = self._service_output(service) + # TEI -> JSON/Markdown conversion only makes sense for TEI services. + tei_service = service not in self.SERVICE_OUTPUTS + # we use ThreadPoolExecutor and not ProcessPoolExecutor because it is an I/O intensive process with concurrent.futures.ThreadPoolExecutor(max_workers=n) as executor: # with concurrent.futures.ProcessPoolExecutor(max_workers=n) as executor: results = [] for input_file in input_files: - # check if TEI file is already produced - filename = self._output_file_name(input_file, input_path, output) + # check if the output file is already produced + filename = self._output_file_name(input_file, input_path, output, output_suffix) if not force and os.path.isfile(filename): self.logger.info( f"{filename} already exists, skipping... (use --force to reprocess pdf input files)") skipped_count += 1 # Check if JSON output is needed but JSON file doesn't exist - if json_output: + if tei_service and json_output: json_filename = filename.replace('.grobid.tei.xml', '.json') # Expand ~ to home directory before checking file existence json_filename_expanded = os.path.expanduser(json_filename) @@ -551,7 +628,7 @@ def process_batch( self.logger.error(f"Failed to convert TEI to JSON for {filename}: {str(e)}") # Check if Markdown output is needed but Markdown file doesn't exist - if markdown_output: + if tei_service and markdown_output: markdown_filename = filename.replace('.grobid.tei.xml', '.md') # Expand ~ to home directory before checking file existence markdown_filename_expanded = os.path.expanduser(markdown_filename) @@ -599,7 +676,7 @@ def process_batch( for r in concurrent.futures.as_completed(results): input_file, status, text = r.result() - filename = self._output_file_name(input_file, input_path, output) + filename = self._output_file_name(input_file, input_path, output, output_suffix) if status != 200 or text is None: self.logger.error(f"Processing of {input_file} failed with error {status}: {text}") @@ -607,10 +684,14 @@ def process_batch( # writing error file with suffixed error code try: pathlib.Path(os.path.dirname(filename)).mkdir(parents=True, exist_ok=True) - error_filename = filename.replace(".grobid.tei.xml", f"_{status}.txt") + if filename.endswith(output_suffix): + error_filename = filename[:-len(output_suffix)] + f"_{status}.txt" + else: + error_filename = filename + f"_{status}.txt" with open(error_filename, 'w', encoding='utf8') as error_file: + # Error responses are always text, even for binary services if text is not None: - error_file.write(text) + error_file.write(text if isinstance(text, str) else text.decode('utf-8', 'replace')) else: error_file.write("") self.logger.info(f"Error details written to {error_filename}") @@ -618,15 +699,21 @@ def process_batch( self.logger.error(f"Failed to write error file {filename}: {str(e)}") else: processed_count += 1 - # writing TEI file + # writing output file try: pathlib.Path(os.path.dirname(filename)).mkdir(parents=True, exist_ok=True) - with open(filename, 'w', encoding='utf8') as tei_file: - tei_file.write(text) - self.logger.debug(f"Successfully wrote TEI file: {filename}") - + if binary_output: + # e.g. annotatePDF returns an annotated PDF (bytes) + payload = text if isinstance(text, (bytes, bytearray)) else str(text).encode('utf-8') + with open(filename, 'wb') as output_file_handle: + output_file_handle.write(payload) + else: + with open(filename, 'w', encoding='utf8') as tei_file: + tei_file.write(text) + self.logger.debug(f"Successfully wrote output file: {filename}") + # Convert to JSON if requested - if json_output: + if tei_service and json_output: try: converter = TEI2LossyJSONConverter() json_data = converter.convert_tei_file(filename, stream=False) @@ -644,7 +731,7 @@ def process_batch( self.logger.error(f"Failed to convert TEI to JSON for {filename}: {str(e)}") # Convert to Markdown if requested - if markdown_output: + if tei_service and markdown_output: try: from .format.TEI2Markdown import TEI2MarkdownConverter converter = TEI2MarkdownConverter() @@ -663,7 +750,7 @@ def process_batch( self.logger.error(f"Failed to convert TEI to Markdown for {filename}: {str(e)}") except OSError as e: - self.logger.error(f"Failed to write TEI XML file {filename}: {str(e)}") + self.logger.error(f"Failed to write output file {filename}: {str(e)}") # Calculate batch statistics batch_runtime = time.time() - batch_start_time @@ -691,7 +778,14 @@ def process_pdf( flavor: Optional[str] = None, start: int = -1, end: int = -1 - ) -> Tuple[str, int, Optional[str]]: + ) -> Tuple[str, int, Union[str, bytes, None]]: + """Send a PDF to GROBID and return (file, status, response). + + The response is the TEI XML as text for the ``process*`` services, JSON + as text for the annotation services, and raw bytes for ``annotatePDF``, + which answers with an annotated PDF rather than a document description. + See https://github.com/grobidOrg/grobid-client-python/issues/79 + """ pdf_handle = None try: pdf_handle = open(pdf_file, "rb") @@ -730,8 +824,9 @@ def process_pdf( if end and end > 0: the_data["end"] = str(end) + accept_header, _, binary_output = self._service_output(service) res, status = self.post( - url=the_url, files=files, data=the_data, headers={"Accept": "text/plain"}, + url=the_url, files=files, data=the_data, headers={"Accept": accept_header}, timeout=self.config['timeout'] ) @@ -753,8 +848,12 @@ def process_pdf( end ) + # Binary services (e.g. annotatePDF) return raw bytes on success; + # error responses are always text. + if binary_output and status == 200: + return (pdf_file, status, res.content) return (pdf_file, status, res.text) - + except IOError as e: self.logger.error(f"Failed to open PDF file {pdf_file}: {str(e)}") return (pdf_file, 400, f"Failed to open file: {str(e)}") @@ -852,7 +951,12 @@ def main() -> None: "processReferences", "processCitationList", "processCitationPatentST36", - "processCitationPatentPDF" + "processCitationPatentPDF", + # PDF annotation services (see issue #79). These return JSON coordinates + # or an annotated (binary) PDF rather than TEI XML. + "referenceAnnotations", + "annotatePDF", + "citationPatentAnnotations" ] parser = argparse.ArgumentParser(description="Client for GROBID services") diff --git a/tests/test_client.py b/tests/test_client.py index 1fe15ee..6658f73 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -116,6 +116,25 @@ def test_call_api_success(self, mock_request): assert response == mock_response assert status == 200 + @patch('grobid_client.client.requests.request') + def test_call_api_keeps_the_caller_accept_header(self, mock_request): + """A caller asking for a representation must actually get it. + + The annotation services (issue #79) need JSON or an annotated PDF back, + so the class-wide accept type is only the fallback. + """ + mock_response = Mock() + mock_response.status_code = 200 + mock_request.return_value = mock_response + + self.client.call_api( + method="POST", + url="http://test.com/api", + headers={"Accept": "application/pdf"}, + ) + + assert mock_request.call_args[1]['headers']['Accept'] == 'application/pdf' + @patch('grobid_client.client.requests.request') def test_get_method(self, mock_request): """Test GET method.""" diff --git a/tests/test_grobid_client.py b/tests/test_grobid_client.py index 7dd6ec3..c9ed15c 100644 --- a/tests/test_grobid_client.py +++ b/tests/test_grobid_client.py @@ -671,3 +671,164 @@ def test_get_server_url_edge_cases(self, mock_configure_logging, mock_test_serve result = client.get_server_url(service) expected = 'http://localhost:8070/api/processCitationPatentST36' assert result == expected + + +class TestAnnotationServices: + """Tests for the PDF annotation services (issue #79).""" + + def _client(self): + with patch('grobid_client.grobid_client.GrobidClient._test_server_connection'): + with patch('grobid_client.grobid_client.GrobidClient._configure_logging'): + client = GrobidClient(check_server=False) + client.logger = Mock() + return client + + def test_service_output_defaults_to_tei(self): + """Unknown/regular services fall back to the TEI output descriptor.""" + client = self._client() + assert client._service_output('processFulltextDocument') == \ + ('application/xml', '.grobid.tei.xml', False) + + def test_service_output_annotation_services(self): + """Annotation services expose their own Accept header/suffix/binary flag.""" + client = self._client() + assert client._service_output('referenceAnnotations') == \ + ('application/json', '.references.json', False) + assert client._service_output('citationPatentAnnotations') == \ + ('application/json', '.patent-citations.json', False) + assert client._service_output('annotatePDF') == \ + ('application/pdf', '.annotated.pdf', True) + + def test_output_file_name_with_custom_suffix(self): + """The output suffix is honoured when building the output file name.""" + client = self._client() + result = client._output_file_name( + '/input/document.pdf', '/input', '/output', '.references.json') + assert result == '/output/document.references.json' + + @patch('builtins.open', new_callable=mock_open) + @patch('grobid_client.grobid_client.GrobidClient.post') + def test_process_pdf_json_annotation_uses_json_accept(self, mock_post, mock_file): + """referenceAnnotations requests JSON and returns the response text.""" + mock_response = Mock() + mock_response.text = '{"refs": []}' + mock_post.return_value = (mock_response, 200) + + client = self._client() + result = client.process_pdf( + 'referenceAnnotations', '/test/document.pdf', + generate_ids=False, consolidate_header=False, consolidate_citations=False, + include_raw_citations=False, include_raw_affiliations=False, + tei_coordinates=False, segment_sentences=False) + + assert mock_post.call_args.kwargs['headers']['Accept'] == 'application/json' + assert result == ('/test/document.pdf', 200, '{"refs": []}') + + @patch('builtins.open', new_callable=mock_open) + @patch('grobid_client.grobid_client.GrobidClient.post') + def test_process_pdf_annotate_returns_binary_content(self, mock_post, mock_file): + """annotatePDF requests a PDF and returns raw bytes on success.""" + mock_response = Mock() + mock_response.content = b'%PDF-annotated' + mock_response.text = 'should not be used' + mock_post.return_value = (mock_response, 200) + + client = self._client() + result = client.process_pdf( + 'annotatePDF', '/test/document.pdf', + generate_ids=False, consolidate_header=False, consolidate_citations=False, + include_raw_citations=False, include_raw_affiliations=False, + tei_coordinates=False, segment_sentences=False) + + assert mock_post.call_args.kwargs['headers']['Accept'] == 'application/pdf' + assert result == ('/test/document.pdf', 200, b'%PDF-annotated') + + @patch('builtins.open', new_callable=mock_open) + @patch('grobid_client.grobid_client.GrobidClient.post') + def test_process_pdf_annotate_error_returns_text(self, mock_post, mock_file): + """On error, annotatePDF returns the response text (not bytes).""" + mock_response = Mock() + mock_response.content = b'binary' + mock_response.text = 'error detail' + mock_post.return_value = (mock_response, 500) + + client = self._client() + result = client.process_pdf( + 'annotatePDF', '/test/document.pdf', + generate_ids=False, consolidate_header=False, consolidate_citations=False, + include_raw_citations=False, include_raw_affiliations=False, + tei_coordinates=False, segment_sentences=False) + + assert result == ('/test/document.pdf', 500, 'error detail') + + def test_warn_unsupported_params_for_annotation_service(self): + """Options ignored by an annotation service are reported once.""" + client = self._client() + client.logger = Mock() + client._warn_unsupported_service_params('annotatePDF', { + 'consolidate_header': True, + 'consolidate_citations': True, + 'tei_coordinates': True, + }) + client.logger.warning.assert_called_once() + msg = client.logger.warning.call_args[0][0] + assert 'annotatePDF' in msg + assert 'consolidate_header' in msg + assert 'tei_coordinates' in msg + # supported option must not be listed + assert 'consolidate_citations' not in msg + + def test_warn_unsupported_params_none_when_all_supported(self): + """No warning when only supported options are set.""" + client = self._client() + client.logger = Mock() + client._warn_unsupported_service_params('referenceAnnotations', { + 'consolidate_citations': True, + 'include_raw_citations': True, + 'tei_coordinates': False, + }) + client.logger.warning.assert_not_called() + + def test_warn_unsupported_params_skipped_for_regular_service(self): + """Regular (non-annotation) services never trigger the warning.""" + client = self._client() + client.logger = Mock() + client._warn_unsupported_service_params('processFulltextDocument', { + 'consolidate_header': True, + 'tei_coordinates': True, + 'segment_sentences': True, + }) + client.logger.warning.assert_not_called() + + @patch('builtins.open', new_callable=mock_open) + @patch('grobid_client.client.requests.request') + def test_accept_header_reaches_the_request(self, mock_request, mock_file): + """The per-service Accept header must survive down to the actual request. + + The other tests here mock post() and assert on what process_pdf passes, + which is one layer above the request: call_api used to overwrite Accept + with its class-wide accept_type, so every service went out as + application/xml and the annotation endpoints were asked for the wrong + representation. + """ + mock_response = Mock() + mock_response.status_code = 200 + mock_response.text = '{}' + mock_response.content = b'%PDF' + mock_request.return_value = mock_response + + client = self._client() + sent = {} + for service in ('processFulltextDocument', 'referenceAnnotations', 'annotatePDF'): + client.process_pdf( + service, '/test/document.pdf', + generate_ids=False, consolidate_header=False, consolidate_citations=False, + include_raw_citations=False, include_raw_affiliations=False, + tei_coordinates=False, segment_sentences=False) + sent[service] = mock_request.call_args[1]['headers']['Accept'] + + assert sent == { + 'processFulltextDocument': 'application/xml', + 'referenceAnnotations': 'application/json', + 'annotatePDF': 'application/pdf', + }