search

valeriupredoi / bgcval2

Package for BGCVal v2.0
3 stars 0 forks source link

Fix shelve opens to "with"style opening #126

Closed ledm closed 3 months ago

ledm commented 3 months ago

This should hopefully settle some of the shelve issues, described in issue #125, but does not close it, or replace them with json files.

ledm commented 3 months ago

I've also gone though the code and changed all instances of shopen to shOpen.

ledm commented 3 months ago

you left a whole lot of the old open style handlers without context managers, I reckon you still working on this, bud?

Yes, but a lot of the code doesn't really get used. I'm also not sure that having shOpen inside a try/except loop is a good idea. Wouldn't it be better to just break so you know its going wrong?

valeriupredoi commented 3 months ago

ah gotcha - I missed that some are not used; yeah no, you're right - a try/except block should not suppress errors from opening, I thought those were catching issues related to stuff after the open, and that is actually bad since if you're stepping over an IO issue related to an open that should indeed be raised. I should have been more careful looking at the code, soz, bud

ledm commented 3 months ago

Okay, I've removed the try loops for shOpen and redone all the shOpen commands that actually get used. Think this is ready for a review and merge @valeriupredoi. Thanks!

ledm commented 3 months ago

Apologies for the single commit push.

ledm commented 3 months ago

Okay, issues resolved, tests passed, the code runs fine on jasmin. Happy to merge.

valeriupredoi commented 3 months ago

Go for it, bud 🍺