python 3 compatibility

[DirkEilander]

This week wflow has been upgraded to version 2018.1 which support only Python 3.6+, and PCRaster 4.2+ and is not backwards compatible to Python 2.7 because of PCRaster 4.1. Now PCRGLOB-WB and wflow cannot both be supported by GLOFRIM anymore as they require different versions of Python and PCRaster. @ChippChapp Any idea whether PCRGLOB-WB is also moving towards Python 3.6+?

Either way we need to start thinking about Python 3.6+ support too as the whole Python environment is moving fast in that direction. Perhaps maintaining a branch for PCRGLOB-WB support for a little while. While this is not the most urgent issue, we should tackle this soon after the v2 release.

[JannisHoch]

hi @DirkEilander that's indeed one of the issues I have in my head but honestly I don't know whether there are any efforts at UU towards moving PCR-GLOBWB to python 3... Since PCRaster and PCR-GLOBWB are both from Physical Geography, one may think that this should be in the pipeline, but in the end they are two different projects... I'll ask!

Guess that we need to maintain a python 2 branch indeed until everything runs stable with python 3.

• [ ] make cristal clear statement in docs that WFLOW 2017.01 must be used and not 2018.1

[JannisHoch]

I have checked and apparently PCRaster can be compiled against Python 3 on Linux and also PCR-GLOBWB should work with Python 3. Hasn't really been tested yet though...

[DirkEilander]

In that case i'm pretty sure it's not going to work. ;) I heard from Deltares colleagues that PCRaster4.2 (the version that works with python 3+) is much more strict on datatypes. It's very likely that some refactoring of the PCR-GLOBWB code needs to be done before that works, but once v2 is done we should create a python3 branch and start testing.

[JannisHoch]

Haha, you may be right about that. Let's worry about this issue another time :)

[JannisHoch]

@DirkEilander I have worked on the migration to Python 3. Test A for the Amazon works fine (PCR->CMF) after changing some code, mostly built-in changes in Python fuctions.

Test B breaks when coupling 2D->1D with the error message

IndexError: too many indices for array

I guess it has to do with this function, maybe the d.items works in a different wah than d.iteritems?

def dictinvert(d):

inv = {}
# for k, v in d.iteritems(): # old python 2 style
for k, v in d.items():
keys = inv.setdefault(v, [])
keys.append(k)

return inv

Would you mind checking and whether you can reproduce this with the migrate_to_python3 branch and the corresponding PCR_BMI branch?

[DirkEilander]

I believe @hcwinsemius has already solved this. Could you confirm and push your latest version Hessel? Are you aware of each others work on migrating this to py3?

[JannisHoch]

@DirkEilander @hcwinsemius from what i see hessel just worked on the lfp_bmi.py and wfl_bmi.py files, not in the spatial_coupling.py where the error occurs... The version I work with already contains all of Hessel's commits so far. If you have the solution locally, then indeed please commit and push :)

@hcwinsemius did you manage to run test B succesfully from the supplement material of the NHESS article?

[DirkEilander]

@JannisHoch @hcwinsemius In general i think we're quit far with this transformation, thanks to you both!

There's still a breaking issue in a recent commit, which i think we can fix easily but i'd like to get Hessel's comments first, see https://github.com/openearth/glofrim/commit/d2671054385132d3b5c92a897d756a0b842f6ea1#commitcomment-39179240

After this is settled and we comfirmed all cases in the test repo are working i'm happy to push this to the master branch

[JannisHoch]

@DirkEilander and @hcwinsemius thanks for checking. before the most recent commit's (i.e. more or less a month ago I guess, but no clue when exactly), I was able to run the PCR->CMF->LFP cascade for test B (Ganges). With the latest commits, it breaks as Dirk already mentioned. Let's wait for @hcwinsemius to respond to Dirks comment and see from there.

P.S. I don't have any uncommited changes locally.