modflowpy / flopy

A Python package to create, run, and post-process MODFLOW-based models.
https://flopy.readthedocs.io
Other
506 stars 306 forks source link

refactor(datafile): ignore "text" parameter, add attributes from file #2231

Open mwtoews opened 2 months ago

mwtoews commented 2 months ago

This PR ignores the text= parameter from HeadFile, HeadUFile and UcnFile, since this should be read from the file and is constant for all records. This PR adds these attributes:

There are instances where other binary files can be attempted to be read (e.g. "mt3d_test/mf96mt3d/P01/case1b/MT3D001.UCN" with HeadFile), but these are now caught with exceptions raised. Warnings are raised if the text records are inconsistent, which shouldn't happen (unless I've misunderstood something!)

mwtoews commented 2 months ago

This does have a breaking change, where previously the obj.text attribute was bytes, and now changed to str. For instance:

Before:

>>> pth = example_data_path / "unstructured/headu.githds"
>>> obj = flopy.utils.binaryfile.HeadUFile(pth)
>>> obj.text
b'headu'

After:

>>> obj = flopy.utils.binaryfile.HeadUFile(pth)
>>> obj.text
'headu'
>>> obj.text_bytes
b'           HEADU'

if this is a deal-breaker, then obj.text can be kept as the modified bytes (possibly raising a deprecation message if accessed), and the str version of the attribute instead stored at obj.text_str.

Also, I should note that this PR does not change FormattedHeadFile, since further refactoring may be needed.

codecov[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 63.15789% with 7 lines in your changes missing coverage. Please review.

Project coverage is 70.8%. Comparing base (e2a85a3) to head (fb14357).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #2231 +/- ## ========================================= - Coverage 70.8% 70.8% -0.1% ========================================= Files 294 294 Lines 59026 59029 +3 ========================================= - Hits 41832 41804 -28 - Misses 17194 17225 +31 ``` | [Files](https://app.codecov.io/gh/modflowpy/flopy/pull/2231?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=modflowpy) | Coverage Δ | | |---|---|---| | [flopy/export/vtk.py](https://app.codecov.io/gh/modflowpy/flopy/pull/2231?src=pr&el=tree&filepath=flopy%2Fexport%2Fvtk.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=modflowpy#diff-ZmxvcHkvZXhwb3J0L3Z0ay5weQ==) | `78.0% <100.0%> (-0.1%)` | :arrow_down: | | [flopy/utils/binaryfile.py](https://app.codecov.io/gh/modflowpy/flopy/pull/2231?src=pr&el=tree&filepath=flopy%2Futils%2Fbinaryfile.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=modflowpy#diff-ZmxvcHkvdXRpbHMvYmluYXJ5ZmlsZS5weQ==) | `81.7% <100.0%> (+0.1%)` | :arrow_up: | | [flopy/utils/datafile.py](https://app.codecov.io/gh/modflowpy/flopy/pull/2231?src=pr&el=tree&filepath=flopy%2Futils%2Fdatafile.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=modflowpy#diff-ZmxvcHkvdXRpbHMvZGF0YWZpbGUucHk=) | `73.6% <100.0%> (+0.3%)` | :arrow_up: | | [flopy/mf6/utils/binaryfile\_utils.py](https://app.codecov.io/gh/modflowpy/flopy/pull/2231?src=pr&el=tree&filepath=flopy%2Fmf6%2Futils%2Fbinaryfile_utils.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=modflowpy#diff-ZmxvcHkvbWY2L3V0aWxzL2JpbmFyeWZpbGVfdXRpbHMucHk=) | `10.6% <0.0%> (ø)` | | | [flopy/export/utils.py](https://app.codecov.io/gh/modflowpy/flopy/pull/2231?src=pr&el=tree&filepath=flopy%2Fexport%2Futils.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=modflowpy#diff-ZmxvcHkvZXhwb3J0L3V0aWxzLnB5) | `59.0% <16.6%> (-2.4%)` | :arrow_down: | ... and [6 files with indirect coverage changes](https://app.codecov.io/gh/modflowpy/flopy/pull/2231/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=modflowpy)
mwtoews commented 2 months ago

Thanks @langevin-usgs for the info, I'll create and add a mixed text example to test, e.g. head/drawdown. The text parameter can be changed to optional, which would default to read the first text header, and warn if more than one found.

mwtoews commented 2 months ago

Confirmed I can create the mixed-text file with older MF versions, and testing current and ancient versions of flopy I can see this feature has never worked. I'll superseded this PR with a fix after a few other related refactors that use the .headers data frame property.