Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle BOMs when loading HRA criteria tables #1461

Merged
merged 6 commits into from
Dec 11, 2023
Merged

Handle BOMs when loading HRA criteria tables #1461

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
753bcb3
Fixing and testing UTF-8-SIG for the criteria table.
phargogh Dec 5, 2023
dcf897b
Noting change in HISTORY.
phargogh Dec 5, 2023
3c07a04
Merge branch 'main' of https://github.com/natcap/invest into bugfix/1…
phargogh Dec 5, 2023
b19b946
Using utf-8-sig encoding directly.
phargogh Dec 7, 2023
4dbe7f5
Making sure the new file is closed after we're done with it. RE:#1460
phargogh Dec 7, 2023
0bc66be
Doing the test by opening the file in binary mode. RE:#1460
phargogh Dec 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions HISTORY.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,10 @@ Unreleased Changes
table columns: ``cur_path``, ``fut_path``, and ``base_path`` are meant
to be file system path strings.
https://github.com/natcap/invest/issues/1455
* HRA
* Fixed an issue preventing the HRA criteria table from loading when the
table was UTF-8 encoded with a Byte-Order Marker.
https://github.com/natcap/invest/issues/1460
* NDR
* Fixing an issue where minor geometric issues in the watersheds input
(such as a ring self-intersection) would raise an error in the model.
Expand Down
35 changes: 35 additions & 0 deletions tests/test_hra.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,41 @@ def test_criteria_table_parsing(self):
pandas.testing.assert_frame_equal(
expected_composite_dataframe, composite_dataframe)

def test_criteria_table_parsing_with_bom(self):
"""HRA: criteria table - parse a BOM."""
from natcap.invest import hra

criteria_table_path = os.path.join(self.workspace_dir, 'criteria.csv')
with open(criteria_table_path, 'w', encoding='utf-8-sig') as criteria_table:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utf-8-sig includes the BOM when writing the file. Cool!

criteria_table.write(
textwrap.dedent(
"""\
HABITAT NAME,eelgrass,,,hardbottom,,,CRITERIA TYPE
HABITAT RESILIENCE ATTRIBUTES,RATING,DQ,WEIGHT,RATING,DQ,WEIGHT,E/C
recruitment rate,2,2,2,2,2,2,C
connectivity rate,2,2,2,2,2,2,C
,,,,,,,
HABITAT STRESSOR OVERLAP PROPERTIES,,,,,,,
oil,RATING,DQ,WEIGHT,RATING,DQ,WEIGHT,E/C
frequency of disturbance,2,2,3,2,2,3,C
management effectiveness,2,2,1,2,2,1,E
,,,,,,,
fishing,RATING,DQ,WEIGHT,RATING,DQ,WEIGHT,E/C
frequency of disturbance,2,2,3,2,2,3,C
management effectiveness,2,2,1,2,2,1,E
"""
))

# Sanity check: make sure the file has the expected BOM
bom_char = "\uFEFF" # byte-order marker in 16-bit hex value
with open(criteria_table_path) as criteria_table:
assert criteria_table.read().startswith(bom_char)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This bit is still failing on Windows and I think it's because of the following from Python's codec docs (https://docs.python.org/3/library/codecs.html#encodings-and-unicode)

On encoding the utf-8-sig codec will write 0xef, 0xbb, 0xbf as the first three bytes to the file. On decoding utf-8-sig will skip those three bytes if they appear as the first three bytes in the file. In UTF-8, the use of the BOM is discouraged and should generally be avoided.

So maybe it's being stripped out when opening and decoding? Although this is contradictory to what I'm seeing in my terminal:

>>> with open("test-bom.csv", 'w', encoding='utf-8-sig') as my_table:
...     my_table.write("HABITAT NAME,eelgrass,,,hardbottom,,,CRITERIA TYPE")
>>>
>>> with open("test-bom.csv") as table:
...     print(table.read())
...
HABITAT NAME,eelgrass,,,hardbottom,,,CRITERIA TYPE

Which makes sense because:

To increase the reliability with which a UTF-8 encoding can be detected, Microsoft invented a variant of UTF-8 (that Python calls "utf-8-sig") for its Notepad program: Before any of the Unicode characters is written to the file, a UTF-8 encoded BOM (which looks like this as a byte sequence: 0xef, 0xbb, 0xbf) is written. As it’s rather improbable that any charmap encoded file starts with these byte values (which would e.g. map to

LATIN SMALL LETTER I WITH DIAERESIS
RIGHT-POINTING DOUBLE ANGLE QUOTATION MARK
INVERTED QUESTION MARK

So, I'm not sure why the test assertion error looks like the BOM is being decoded and stripped, where as when I open and read it I get the funky character mapping...


target_composite_csv_path = os.path.join(self.workspace_dir,
'composite.csv')
hra._parse_criteria_table(criteria_table_path,
target_composite_csv_path)

def test_criteria_table_file_not_found(self):
"""HRA: criteria table - spatial file not found."""
from natcap.invest import hra
Expand Down
Loading