Closed ColeDCrawford closed 5 months ago
I think maybe we should catch the error and process in the same way here: https://github.com/ixc/python-edtf/blob/v5/edtf/fields.py#L135-L138
What do you think about this approach?
diff --git a/edtf/fields.py b/edtf/fields.py
index f717592..2f25c94 100644
--- a/edtf/fields.py
+++ b/edtf/fields.py
@@ -4,10 +4,12 @@ from django.core.exceptions import FieldDoesNotExist
from django.db import models
from django.db.models import signals
from django.db.models.query_utils import DeferredAttribute
+from pyparsing import ParseException
from edtf import EDTFObject, parse_edtf
from edtf.convert import struct_time_to_date, struct_time_to_jd
from edtf.natlang import text_to_edtf
+from edtf.parser.edtf_exceptions import EDTFParseException
DATE_ATTRS = (
"lower_strict",
@@ -132,10 +134,12 @@ class EDTFField(models.CharField):
if direct_input and (
existing_value is None or str(existing_value) != direct_input
):
- edtf = parse_edtf(
- direct_input, fail_silently=True
- ) # ParseException if invalid; should this be raised?
- # TODO pyparsing.ParseExceptions are very noisy and dumps the whole grammar (see https://github.com/ixc/python-edtf/issues/46)
+ try:
+ edtf = parse_edtf(
+ direct_input, fail_silently=True
+ ) # ParseException if invalid; should this be raised?
+ except ParseException as err:
+ raise EDTFParseException(direct_input, err) from None
# set the natural_text (display) field to the direct_input if it is not provided
if natural_text == "":
diff --git a/edtf/parser/edtf_exceptions.py b/edtf/parser/edtf_exceptions.py
index 9530602..2a2dc18 100644
--- a/edtf/parser/edtf_exceptions.py
+++ b/edtf/parser/edtf_exceptions.py
@@ -2,4 +2,22 @@ from pyparsing import ParseException
class EDTFParseException(ParseException):
- pass
+ """Raised when an input cannot be parsed as an EDTF string.
+
+ Attributes:
+ input_string - the input string that could not be parsed
+ err -- the original ParseException that caused this one
+ """
+
+ def __init__(self, input_string, err):
+ self.input_string = input_string
+ self.err = err
+ super().__init__(err)
+
+ def __str__(self):
+ if not self.input_string or not isinstance(self.err, ParseException):
+ return super().__str__()
+ near_text = ""
+ near_text = self.input_string[max(self.err.loc - 10, 0) : self.err.loc + 10]
+ full_msg = f"Error at position {self.err.loc}: Invalid input or format near '{near_text}'. Please provide a valid EDTF string."
+ return full_msg
diff --git a/edtf/parser/grammar.py b/edtf/parser/grammar.py
index 773f806..701d398 100644
--- a/edtf/parser/grammar.py
+++ b/edtf/parser/grammar.py
@@ -357,8 +357,4 @@ def parse_edtf(input_string, parseAll=True, fail_silently=False, debug=None):
return None
if debug:
raise
- near_text = ""
- if input_string:
- near_text = input_string[max(err.loc - 10, 0) : err.loc + 10]
- full_msg = f"Error at position {err.loc}: Invalid input or format near '{near_text}'. Please provide a valid EDTF string."
- raise EDTFParseException(full_msg) from None
+ raise EDTFParseException(input_string, err) from None
Thanks for the feedback @aweakley - let me know if there's anything else needed here.
I think these are failing due a permissions issue again - the tests themselves seem to pass.
I'm seeing an error with the coverage report in my latest run here: https://github.com/ixc/python-edtf/actions/runs/9358234831/job/25820224297#step:11:1
Run MishaKav/pytest-coverage-comment@main
File read successfully "/home/runner/work/python-edtf/python-edtf/./coverage_combined.xml"
Generating coverage report
File read successfully "/home/runner/work/python-edtf/python-edtf/./combined_junit_pytest.xml"
File read successfully "/home/runner/work/python-edtf/python-edtf/./combined_junit_pytest.xml"
errors: 0
failures: 0
skipped: 0
tests: 258
time: 2.649
File read successfully "/home/runner/work/python-edtf/python-edtf/./combined_junit_pytest.xml"
Warning: Your comment is too long (maximum is 65536 characters), coverage report will not be added.
Warning: Try add: "--cov-report=term-missing:skip-covered", or add "hide-report: true", or add "report-only-changed-files: true", or switch to "multiple-files" mode
File read successfully "/home/runner/work/python-edtf/python-edtf/./combined_junit_pytest.xml"
./coverage_combined.xml
No previous comment found, creating a new one
Error: HttpError: Resource not accessible by integration
Error: Resource not accessible by integration
I think this is the same permissions issue as in PRs like this:
I think this is because the PRs are from a fork: https://github.com/MishaKav/pytest-coverage-comment/issues/68
If you look at the workflow on my fork, you can see all the tests work and everything runs up until publishing the benchmarks, which fails because I don't have GHPages set up for the fork: https://github.com/artshumrc/python-edtf/actions/runs/9358234517/job/25759630204
Thank you, this looks great.
Any other ideas of what we would want to improve here @aweakley ?