-
Notifications
You must be signed in to change notification settings - Fork 10
Using private grid._depth for negating depth #75
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.
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
Conversation
As Parcels v3.1.0 has moved from `grid.depth` to `grid._depth`, the code for VirtualShip is now broken. THis PR fixes that, although a more propoer implementation would be to add a grid.negate_depth() method to Parcels itself?
for more information, see https://pre-commit.ci
|
The other issue that causes the CI to fail is not easily fixed within virtualship; it has to do with a bug in a new warning in Parcels, see Parcels-code/Parcels#1747. In any case, because it's 'only a warning', it doesn't affect the functionality of virtualship. We probably have to wait until Parcels is patched, @VeckoTheGecko? |
Using private grid._depth for negating depth (Parcels-code#75)
|
@erikvansebille I wonder if a better solution would be to pin parcels<3.1.0 for the timebeing? That way we can implement |
|
Yes I agree; pinning to <3.1.0 (or even 'not 3.1.0'; is that possible?) might be a better solution. Can you implement? |
|
Just so you know. There is no rush to fix things for virtual ship. I plan to have the next group of students working with the code early February. |
This reverts commit 6f843e9.
Temporarily. #75 (comment)
This reverts commit 6f843e9.
Temporarily. #75 (comment)
As Parcels v3.1.0 has moved from
grid.depthtogrid._depth, the code for VirtualShip is now broken.This PR fixes that, although a more proper implementation would be to add a
grid.negate_depth()method to Parcels itself?