Copy over the required shapefile files upon saving forcing #457
Conversation
This file contains 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
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #457 +/- ##
==========================================
- Coverage 78.93% 78.83% -0.10%
==========================================
Files 28 28
Lines 1804 1810 +6
Branches 216 218 +2
==========================================
+ Hits 1424 1427 +3
- Misses 323 325 +2
- Partials 57 58 +1
|
|
@sverhoeven shall we make a new release for this as well? I.e. 2.3.1 |
Sure, do you think we need something else here to fix the tests in the ewatercycle-* packages? If so we might want to delay the release. |
The tests are now only failing due to changes in the output format of the Forcing objects (which they already did before this PR). There are some other things we should tackle such as #423, but that's not a priority now. |
sverhoeven
approved these changes
Sep 10, 2024
There was a problem hiding this comment.
Tested with
from ewatercycle.testing.fixtures import rhine_shape
from ewatercycle.forcing import sources
from pathlib import Path
fc = sources['CaravanForcing']
s = rhine_shape()
fd = Path('/tmp/cf')
fd.mkdir()
f = fc.generate(start_time="1991-01-01T00:00:00Z", end_time="1991-12-31T00:00:00Z", shape=s, directory=fd, basin_id='camels_01022500')
f2= fc.load('/tmp/cf')
f2.shapeand
ls /tmp/cf
camels_01022500_1991-01-01_1991-12-31_evspsblpot.nc camels_01022500_1991-01-01_1991-12-31_tasmax.nc ewatercycle_forcing.yaml Rhine.shp
camels_01022500_1991-01-01_1991-12-31_pr.nc camels_01022500_1991-01-01_1991-12-31_tasmin.nc Rhine.dbf Rhine.shx
camels_01022500_1991-01-01_1991-12-31_Q.nc camels_01022500_1991-01-01_1991-12-31_tas.nc Rhine.prj
Which looks like we expected.
Some minor suggestions.
| shutil.copy(clone.shape, clone.directory / clone.shape.name) | ||
| ) | ||
| # Also copy other required files: | ||
| for ext in [".dbf", ".shx", ".prj"]: |
There was a problem hiding this comment.
The '.prj' file is optional. Can you check that file exists before copying it?
If I run
from ewatercycle.testing.fixtures import rhine_shape
from ewatercycle.forcing import sources
from pathlib import Path
import shutil
fc = sources['CaravanForcing']
Path('/tmp/shptest').mkdir(exist_ok=True)
s = rhine_shape()
shutil.copy(s, '/tmp/shptest/rhine.shp')
shutil.copy(s.with_suffix('.dbf'), '/tmp/shptest/')
shutil.copy(s.with_suffix('.shx'), '/tmp/shptest/')
fd = Path('/tmp/cf')
fd.mkdir()
f = fc.generate(start_time="1991-01-01T00:00:00Z", end_time="1991-12-31T00:00:00Z", shape='/tmp/shptest/rhine.shp', directory=fd, basin_id='camels_01022500')I get
---------------------------------------------------------------------------
FileNotFoundError Traceback (most recent call last)
Cell In[5], [line 1](vscode-notebook-cell:?execution_count=5&line=1)
----> [1](vscode-notebook-cell:?execution_count=5&line=1) f = fc.generate(start_time="1991-01-01T00:00:00Z", end_time="1991-12-31T00:00:00Z", shape='/tmp/shptest/Rhine.shp', directory=fd, basin_id='camels_01022500')
File ~/git/eWaterCycle/ewatercycle/src/ewatercycle/_forcings/caravan.py:244, in CaravanForcing.generate(cls, start_time, end_time, directory, variables, shape, **kwargs)
[229](https://file+.vscode-resource.vscode-cdn.net/home/verhoes/git/eWaterCycle/ewatercycle/~/git/eWaterCycle/ewatercycle/src/ewatercycle/_forcings/caravan.py:229) ds_basin_time[var].to_netcdf(
[230](https://file+.vscode-resource.vscode-cdn.net/home/verhoes/git/eWaterCycle/ewatercycle/~/git/eWaterCycle/ewatercycle/src/ewatercycle/_forcings/caravan.py:230) Path(directory)
[231](https://file+.vscode-resource.vscode-cdn.net/home/verhoes/git/eWaterCycle/ewatercycle/~/git/eWaterCycle/ewatercycle/src/ewatercycle/_forcings/caravan.py:231) / f"{basin_id}_{start_time_name}_{end_time_name}_{var}.nc"
[232](https://file+.vscode-resource.vscode-cdn.net/home/verhoes/git/eWaterCycle/ewatercycle/~/git/eWaterCycle/ewatercycle/src/ewatercycle/_forcings/caravan.py:232) )
[234](https://file+.vscode-resource.vscode-cdn.net/home/verhoes/git/eWaterCycle/ewatercycle/~/git/eWaterCycle/ewatercycle/src/ewatercycle/_forcings/caravan.py:234) forcing = cls(
[235](https://file+.vscode-resource.vscode-cdn.net/home/verhoes/git/eWaterCycle/ewatercycle/~/git/eWaterCycle/ewatercycle/src/ewatercycle/_forcings/caravan.py:235) directory=Path(directory),
[236](https://file+.vscode-resource.vscode-cdn.net/home/verhoes/git/eWaterCycle/ewatercycle/~/git/eWaterCycle/ewatercycle/src/ewatercycle/_forcings/caravan.py:236) start_time=start_time,
(...)
[242](https://file+.vscode-resource.vscode-cdn.net/home/verhoes/git/eWaterCycle/ewatercycle/~/git/eWaterCycle/ewatercycle/src/ewatercycle/_forcings/caravan.py:242) },
[243](https://file+.vscode-resource.vscode-cdn.net/home/verhoes/git/eWaterCycle/ewatercycle/~/git/eWaterCycle/ewatercycle/src/ewatercycle/_forcings/caravan.py:243) )
--> [244](https://file+.vscode-resource.vscode-cdn.net/home/verhoes/git/eWaterCycle/ewatercycle/~/git/eWaterCycle/ewatercycle/src/ewatercycle/_forcings/caravan.py:244) forcing.save()
[245](https://file+.vscode-resource.vscode-cdn.net/home/verhoes/git/eWaterCycle/ewatercycle/~/git/eWaterCycle/ewatercycle/src/ewatercycle/_forcings/caravan.py:245) return forcing
File ~/git/eWaterCycle/ewatercycle/src/ewatercycle/base/forcing.py:230, in DefaultForcing.save(self)
[228](https://file+.vscode-resource.vscode-cdn.net/home/verhoes/git/eWaterCycle/ewatercycle/~/git/eWaterCycle/ewatercycle/src/ewatercycle/base/forcing.py:228) # Also copy other required files:
[229](https://file+.vscode-resource.vscode-cdn.net/home/verhoes/git/eWaterCycle/ewatercycle/~/git/eWaterCycle/ewatercycle/src/ewatercycle/base/forcing.py:229) for ext in [".dbf", ".shx", ".prj"]:
--> [230](https://file+.vscode-resource.vscode-cdn.net/home/verhoes/git/eWaterCycle/ewatercycle/~/git/eWaterCycle/ewatercycle/src/ewatercycle/base/forcing.py:230) shutil.copy(
[231](https://file+.vscode-resource.vscode-cdn.net/home/verhoes/git/eWaterCycle/ewatercycle/~/git/eWaterCycle/ewatercycle/src/ewatercycle/base/forcing.py:231) clone.shape.with_suffix(ext),
[232](https://file+.vscode-resource.vscode-cdn.net/home/verhoes/git/eWaterCycle/ewatercycle/~/git/eWaterCycle/ewatercycle/src/ewatercycle/base/forcing.py:232) clone.directory / clone.shape.with_suffix(ext).name,
...
[261](https://file+.vscode-resource.vscode-cdn.net/home/verhoes/git/eWaterCycle/ewatercycle/~/miniforge3/envs/ewatercycle/lib/python3.12/shutil.py:261) try:
[262](https://file+.vscode-resource.vscode-cdn.net/home/verhoes/git/eWaterCycle/ewatercycle/~/miniforge3/envs/ewatercycle/lib/python3.12/shutil.py:262) with open(dst, 'wb') as fdst:
[263](https://file+.vscode-resource.vscode-cdn.net/home/verhoes/git/eWaterCycle/ewatercycle/~/miniforge3/envs/ewatercycle/lib/python3.12/shutil.py:263) # macOS
FileNotFoundError: [Errno 2] No such file or directory: '/tmp/shptest/Rhine.prj'which is not something we would like to bother a user with
There was a problem hiding this comment.
.prj files are required for our workflow, as we cannot reliably guess what projection the shapefile is in (probably WGS-84, but who knows). If we guess the projection incorrectly the user will get incorrect results without ever knowing (clearly) why.
Therefore I think it's best to keep it as mandatory. I will add a more clear error.
Co-authored-by: Stefan Verhoeven <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #430
With this change only mandatory shapefile files (.shp, .shx, .dbf) are copied, as well as the projection file (.prj). Other files are not copied (too many possibilities and formats to deal with).
ToDo
In most plugins we will have to update the tests (as the format of the YAML forcing object has changed slightly). No other tests seem to break