Skip to content

Conversation

@ecomodeller
Copy link
Member

@ecomodeller ecomodeller commented Dec 15, 2025

Summary

Fixes a regression bug in v3.0.0 (#866) where MULTIPOLYGON strings accumulate quotes on each read-write cycle.

The MULTIPOLYGON special case (added for performance in the PFS rewrite, related to issue #749) was returning raw strings without stripping quotes, causing:

  • First read: 'MULTIPOLYGON...' (quote preserved)
  • After write-read: ''MULTIPOLYGON...' (quote doubled)
  • Each cycle adds another layer of quotes

Solution

Modified the MULTIPOLYGON fast path to strip quotes like other string values while maintaining the performance optimization.

Testing the fix

Users can install this branch to test the fix:

pip install git+https://github.com/DHI/mikeio.git@fix/pfs_single_quote

Or with uv:

uv pip install git+https://github.com/DHI/mikeio.git@fix/pfs_single_quote

Fixes a regression introduced in the PFS rewrite where MULTIPOLYGON
strings would accumulate quotes on each read-write cycle.

The MULTIPOLYGON special case (added for performance, see issue #749)
was returning the raw value_str without stripping quotes. This caused:
- First read: Shape = 'MULTIPOLYGON...' (quote preserved)
- After write-read: Shape = ''MULTIPOLYGON...' (quote doubled)

The fix maintains the performance optimization (25x faster than v2.6.0)
while properly stripping quotes like other string values.

Performance benchmarks (multipolygon.pfs):
- v2.6.0 (YAML): 110ms
- v3.0.0 (buggy): 6.0ms
- Current fix: 4.4ms ✓

Added comprehensive regression test: test_multipolygon_quote_handling
Copy link
Collaborator

@Havrevoll Havrevoll left a comment

Choose a reason for hiding this comment

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

It works.

@ecomodeller ecomodeller merged commit 411b7f6 into main Dec 17, 2025
8 checks passed
@ecomodeller ecomodeller deleted the fix/pfs_single_quote branch December 17, 2025 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants