fix: make the shapefile encoding retry actually decode latin-1 data - #113
Open
AlexianMasson wants to merge 4 commits into
Open
fix: make the shapefile encoding retry actually decode latin-1 data#113AlexianMasson wants to merge 4 commits into
AlexianMasson wants to merge 4 commits into
Conversation
f-necas
force-pushed
the
fix/shapefile-encoding-detection-2
branch
from
August 25, 2026 09:30
0b156a9 to
ef9b58e
Compare
A .cpg *declares* the .dbf encoding as ASCII text ("ISO-8859-1",
"UTF-8", "ANSI 1252"...), so handing it to chardet always reported
"ascii" -- which decodes nothing and made the retry fail outright.
Parse it instead, and discard a UTF-8/ASCII declaration: detection only
runs after a UTF-8 read has already failed, so honouring such a
declaration would just repeat that read. GDAL already honours a valid
.cpg on the first read, so this branch is reached precisely when the
sidecar is missing, wrong or unrecognised.
Also drop the .dbf sampling: chardet cannot read a .dbf (binary header
-> "encoding: None"), and when it does answer on a larger one it guesses
a plausible-but-wrong codepage (cp1250 for latin-1 data at 0.99
confidence), silently corrupting the text instead of failing.
This is the case the branch was opened for. A zipped latin-1 shapefile with no .cpg (or one that wrongly claims UTF-8) still failed: detection returned "utf-8", so the retry re-ran the read that had just raised. GDAL honours a valid .cpg and the .dbf language driver id by itself, so a shapefile that reaches detection declares neither, and no byte-level detector can pick its codepage -- chardet reads latin-1 records as cp1250 at 0.99 confidence, which would silently corrupt the text. Assume cp1252 instead and log a WARNING naming the guess, so mojibake is traceable rather than silent. Covered end to end with a real latin-1 shapefile, parametrized over PYOGRIO_USE_ARROW: only the Arrow path raises on undecodable text, and Airflow sets it (docker/compose.airflow.yaml) while local runs do not.
The loop was meant to force pyarrow's lazy text validation so a bad encoding would raise inside the try block instead of much later in pandas.to_sql. It never did: - with PYOGRIO_USE_ARROW, gpd.read_file already raises the ArrowException while converting the arrow table to pandas, so the loop is unreachable; - without it, pyogrio hands back correctly decoded text (GDAL recodes from the declared encoding, or from ISO-8859-1), so there is nothing to validate -- and .to_numpy() does not validate anything anyway: it returns object cells untouched, surrogate escapes included. Meanwhile it broke frames with repeated field names, which GDAL does produce: result[label] is a DataFrame there, and DataFrame has no .dtype, so an AttributeError escaped the except clause and aborted the whole ingest for files that read fine before. The widened `except (UnicodeDecodeError, ArrowException)` is what actually makes the encoding retry trigger; it now logs the original error so a non-encoding ArrowException isn't reported as an encoding problem. Also import ArrowException from the public pyarrow namespace.
_detect_file_encoding returned the bare string "utf-8" from four places
and compared against ("utf-8", "ascii") in a fifth. Introduce
_DEFAULT_ENCODING and _UTF8_COMPATIBLE_ENCODINGS next to the existing
_SHAPEFILE_FALLBACK_ENCODING, so the three encoding decisions this module
makes each have a name.
There is no standard-library constant to use instead: codecs exposes BOM
byte sequences but no codec name, sys.getdefaultencoding() names the
codec str.encode() defaults to rather than a file's encoding, and
locale.getpreferredencoding() is environment-dependent and returns a
differently-cased name.
_UTF8_COMPATIBLE_ENCODINGS is matched against codecs.lookup().name, so
its entries have to be in normalized form -- "utf8" is a real codec but
would never compare equal. A test pins that for all three constants.
f-necas
force-pushed
the
fix/shapefile-encoding-detection-2
branch
from
August 25, 2026 10:56
ef9b58e to
f6498f2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up on #107
The encoding retry added there still failed on the case it was opened for: a zipped latin-1 shapefile with no
.cpgsidecar. This branch fixes the three reasons it did.What was broken
The
.cpgsidecar was sniffed instead of parsed. A.cpgdeclares the.dbfencoding as ASCII text (ISO-8859-1,UTF-8,ANSI 1252, ...), so handing it tochardetalways reportedascii— which decodes nothing, and the retry failed outright.Detection could only ever return
utf-8. With no usable.cpg, detection fell through toutf-8and the retry simply re-ran the read that had just raised. No byte-level detector can recover the codepage here:chardetreads latin-1.dbfrecords ascp1250at 0.99 confidence, which would silently corrupt text rather than fail.The string-materialization loop never did what it was for, and broke valid files. It was meant to force pyarrow's lazy text validation inside the
tryblock, but withPYOGRIO_USE_ARROWgpd.read_filealready raises while converting to pandas (loop unreachable), and without it GDAL hands back correctly decoded text (nothing to validate —.to_numpy()validates nothing either). Meanwhile, on frames with repeated field names — which GDAL does produce —result[label]is aDataFrame, which has no.dtype, so anAttributeErrorescaped theexceptclause and aborted the whole ingest for files that read fine before.What changed
.cpgas an encoding name (_parse_cpg_encoding), taking the trailing token soANSI 1252→1252andLDID/87→87resolve viacodecs.lookup(). AUTF-8/ASCIIdeclaration is discarded: detection only runs after a UTF-8 read has failed, so honouring it would repeat that read.cp1252for shapefiles that declare no usable encoding, with a WARNING naming the guess so mojibake is traceable and the fix (add a.cpg) is stated at the source..dbfsampling —chardetcannot read a binary.dbfheader, and on larger ones it confidently guesses the wrong codepage.except (UnicodeDecodeError, ArrowException)is what actually triggers the retry, and the original error is now logged so a non-encodingArrowExceptionisn't reported as an encoding problem.ArrowExceptionis imported from the publicpyarrownamespace._DEFAULT_ENCODING,_UTF8_COMPATIBLE_ENCODINGS,_SHAPEFILE_FALLBACK_ENCODING) instead of repeating"utf-8"in five places. The constants must be normalized codec names to compare equal againstcodecs.lookup().name; a test pins that.Tests
13 new tests in
libs/data_manipulation/tests/test_ingestion.pycovering: loose and zipped shapefiles honouring a.cpg, numeric codepage resolution, unusable/UTF-8 declarations being ignored, thecp1252fallback (loose and zipped), GeoJSON always UTF-8, plain-text sniffing, zips without a shapefile, unreadable files, duplicate column labels, and constant normalization.Files touched:
libs/data_manipulation/src/data_manipulation/ingestion.py,libs/data_manipulation/tests/test_ingestion.py.