OSGeo / grass-addons

GRASS GIS Addons Repository
https://grass.osgeo.org/grass-stable/manuals/addons/
GNU General Public License v2.0
101 stars 152 forks source link

[Bug] r.landscape.evol: f = file(statsout, 'wt') not working anymore in python 3 #156

Open hellik opened 4 years ago

hellik commented 4 years ago

Name of the addon r.landscape.evol

Describe the bug f = file(statsout, 'wt') not working anymore in python 3

To Reproduce

e.g. try to run the script in winGRASS

isaacullah commented 4 years ago

Thanks for the bug report. In fact I am working on a pretty drastic re-write of this module. The new version will break some compatibility, but results should be improved. ETA for this is sometime early in the summer. I will try to ensure that this issue is fixed.

As an aside to the GRASS dev team, what is the preferred method for releasing a drastic, non-backwards compatible update to an addon? Should I give it a new name or should it just be that the older version is replaced? The older version will be deprecated after this update and I would no longer be maintaining it (and I would strongly suggest that people would no longer use it). Thanks! ~Isaac

On Fri, May 1, 2020 at 1:39 PM Helmut Kudrnovsky notifications@github.com wrote:

Name of the addon r.landscape.evol

Describe the bug f = file(statsout, 'wt') not working anymore in python 3

To Reproduce

e.g. try to run the script in winGRASS

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/OSGeo/grass-addons/issues/156, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACDYS5FNKRIM5COJ5VOBHCDRPMXPFANCNFSM4MXLHEHQ .

--


Isaac I. Ullah, PhD Recent papers: Laborscapes and Archaeologies of Sustainability: http://dx.doi.org/10.1558/jma.39327 Water, dust, and loess: co-evolution of landscapes, farming, and human society: https://doi.org/10.1016/j.jaa.2019.101067 Modeling feedbacks between human and natural processes in the land system: https://doi.org/10.5194/esd-9-895-2018 https://doi.org/10.5194/esd-9-895-2018 https://doi.org/10.5194/esd-9-895-2018

petrasovaa commented 4 years ago

It depends on you. In a similar case of r.learn.ml the author added r.learn.ml2 because users wanted the old version for compatibility. You can rename the old one and keep it there for at least some time and upload the new one under the same name if appropriate. Or, if you think users should really just switch to new one, provide guidance how to start using the new one and just update it in git, the history will be there if anyone really wants to run the old one. But in any case it would be good to fix the current one for Python 3, hopefully shouldn't be too much work.

isaacullah commented 4 years ago

Thanks Anna! I'm glad it's flexible. Once I've finished test benching the new version and have some idea about how the output compares to the current version , I'll decide which way to go...

Sent from my📱

On Fri, May 1, 2020, 6:31 PM Anna Petrasova notifications@github.com wrote:

It depends on you. In a similar case of r.learn.ml the author added r.learn.ml2 because users wanted the old version for compatibility. You can rename the old one and keep it there for at least some time and upload the new one under the same name if appropriate. Or, if you think users should really just switch to new one, provide guidance how to start using the new one and just update it in git, the history will be there if anyone really wants to run the old one. But in any case it would be good to fix the current one for Python 3, hopefully shouldn't be too much work.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/OSGeo/grass-addons/issues/156#issuecomment-622649546, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACDYS5GG6Z7CR5PBI4MKC63RPNZX3ANCNFSM4MXLHEHQ .

neteler commented 4 years ago

Would a fix following https://stackoverflow.com/a/28121192/452464 work?

mczalazar commented 4 years ago

Hello. I´m trying to fix the current addon version for Python 3. The problem is with the statsout file. I did some changes in r.landscape.evol.py (line 654). If you have read and write permissions the following sentences works fine. I tried with ‘a+’ and ‘w+’ but don´t work. Should be change 'MAPSET' environment? Thanks you

Make the statsout file with correct column headers

if options["statsout"] == "":
    env = grass.gisenv()
    mapset = env['MAPSET']
    statsout = '%s_%slsevol_stats.csv' % (mapset, prefx)
else:
    statsout = options["statsout"]
if os.path.isfile(statsout):
    #cecilia
    f = open(statsout,'a')
    f.close()
else:
    #cecilia
    f = open(statsout,'w')
    f.write('These statistics are in units ……………')
    f.close()
isaacullah commented 4 years ago

Problem is also probably that it's using Python 2 string substitution, which should also be updated.

Again, a new version is forthcoming, so unless your work is time sensitive, I would wait until the new version is ready.

Sent from my📱

On Sat, May 2, 2020, 7:42 AM mczalazar notifications@github.com wrote:

Hello. I´m trying to fix the current addon version for Python 3. The problem is with the statsout file. I did some changes in r.landscape.evol.py (line 654). If you have read and write permissions the following sentences works fine. I tried with ‘a+’ and ‘w+’ but don´t work. Should be change 'MAPSET' environment? Thanks you

Make the statsout file with correct column headers

if options["statsout"] == "": env = grass.gisenv() mapset = env['MAPSET'] statsout = '%s_%slsevol_stats.csv' % (mapset, prefx) else: statsout = options["statsout"] if os.path.isfile(statsout):

cecilia

f = open(statsout,'a') f.close() else:

cecilia

f = open(statsout,'w') f.write('These statistics are in units ……………') f.close()

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/OSGeo/grass-addons/issues/156#issuecomment-622963991, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACDYS5EPOFOENKIEKWQWKQTRPQWNFANCNFSM4MXLHEHQ .

isaacullah commented 4 years ago

Helmut and all, this is just a quick note to say that I have now uploaded the new version of r.landscape.evol to the addons GitHub repo. For now, the previous version is still available as "r.landscape.evol.old," but the old version will no longer be updated, and as soon as I am certain the new version is stable, I will delete the ".old" version from the repo (you could still get the older code by going back in the commit chain if you wanted to). The new version has been updated quite a bit, and I don't think it's really backwards compatible, but it should be a substantial improvement. I advise to read the new documentation carefully. Please feel free to test if you would like and report any issues via the bug tracker. I will be working on it more this summer, including trying to run some benchmarks between Linux, MacOS, and Windows.

~Isaac

On Sat, May 2, 2020 at 10:42 AM Isaac Ullah isaaciullah@gmail.com wrote:

Problem is also probably that it's using Python 2 string substitution, which should also be updated.

Again, a new version is forthcoming, so unless your work is time sensitive, I would wait until the new version is ready.

Sent from my📱

On Sat, May 2, 2020, 7:42 AM mczalazar notifications@github.com wrote:

Hello. I´m trying to fix the current addon version for Python 3. The problem is with the statsout file. I did some changes in r.landscape.evol.py (line 654). If you have read and write permissions the following sentences works fine. I tried with ‘a+’ and ‘w+’ but don´t work. Should be change 'MAPSET' environment? Thanks you

Make the statsout file with correct column headers

if options["statsout"] == "": env = grass.gisenv() mapset = env['MAPSET'] statsout = '%s_%slsevol_stats.csv' % (mapset, prefx) else: statsout = options["statsout"] if os.path.isfile(statsout):

cecilia

f = open(statsout,'a') f.close() else:

cecilia

f = open(statsout,'w') f.write('These statistics are in units ……………') f.close()

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/OSGeo/grass-addons/issues/156#issuecomment-622963991, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACDYS5EPOFOENKIEKWQWKQTRPQWNFANCNFSM4MXLHEHQ .

--


Isaac I. Ullah, PhD Recent papers: Laborscapes and Archaeologies of Sustainability: http://dx.doi.org/10.1558/jma.39327 Water, dust, and loess: co-evolution of landscapes, farming, and human society: https://doi.org/10.1016/j.jaa.2019.101067 Modeling feedbacks between human and natural processes in the land system: https://doi.org/10.5194/esd-9-895-2018 https://doi.org/10.5194/esd-9-895-2018 https://doi.org/10.5194/esd-9-895-2018

neteler commented 4 years ago

The new version of r.landscape.evol compiles for me but I didn't test it.

Can this issue be closed, @hellik ?

mczalazar commented 4 years ago

Hello, is the new version ready to install from the repository (via g.extension)? I´m ussing Grass 7.8.2 in Windows. Because after I re-install it the r.landscape.evol.py seems has not changes. Thanks

El sáb., 20 de jun. de 2020 a la(s) 11:01, Markus Neteler ( notifications@github.com) escribió:

The new version of r.landscape.evol https://github.com/OSGeo/grass-addons/tree/master/grass7/raster/r.landscape.evol compiles for me but I didn't test it.

Can this issue be closed, @hellik https://github.com/hellik ?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/OSGeo/grass-addons/issues/156#issuecomment-646999693, or unsubscribe https://github.com/notifications/unsubscribe-auth/APNS2YHQUDIITJELU6WTJC3RXS6MZANCNFSM4MXLHEHQ .

neteler commented 4 years ago

You are right, there is still a bug:

Traceback (most recent call last):
  File "C:/msys64/usr/src/grass783/dist.x86_64-w64-mingw32/fools/mkhtml.py", line 266, in <module>
    src_data = read_file(src_file)
  File "C:/msys64/usr/src/grass783/dist.x86_64-w64-mingw32/tools/mkhtml.py", line 158, in read_file
    return decode(s)
  File "C:/msys64/usr/src/grass783/dist.x86_64-w64-mingw32/tools/mkhtml.py", line 66, in decode
    return bytes_.decode(enc)
  File "C:\\\\OS3944~1\\apps\\Python37\lib\encodings\cp1250.py", line 15, in decode
    return codecs.charmap_decode(input,errors,decoding_table)
UnicodeDecodeError: 'charmap' codec can't decode byte 0x88 in position 32941: character maps to <undefined>

From https://wingrass.fsv.cvut.cz/grass78/x86_64/addons/grass-7.8.3/logs/r.landscape.evol.log

Overview: https://wingrass.fsv.cvut.cz/grass78/x86_64/addons/grass-7.8.3/logs/

neteler commented 4 years ago

The updated module now compiles on Windows: https://wingrass.fsv.cvut.cz/grass78/x86_64/addons/grass-7.8.3/logs/r.landscape.evol.log

mczalazar commented 4 years ago

Thanks Markus! Now I was able to update it in wingrass 7.8.3 via g.extension. I will try to use this new update.

El sáb., 25 de jul. de 2020 a la(s) 16:43, Markus Neteler ( notifications@github.com) escribió:

The updated module now compiles on Windows: https://wingrass.fsv.cvut.cz/grass78/x86_64/addons/grass-7.8.3/logs/r.landscape.evol.log

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/OSGeo/grass-addons/issues/156#issuecomment-663893547, or unsubscribe https://github.com/notifications/unsubscribe-auth/APNS2YHDNCFACD3FXGUEUOTR5MYVXANCNFSM4MXLHEHQ .

neteler commented 4 years ago

Thanks for the feedback, closing. In case of further issues pls open a new report.

cmbarton commented 4 years ago

I just tried it. Is the fix now in the master branch for addons? If so, it downloads and compiles. But it still bombs for me when it runs.

Iteration 1 -- step 5/6: calculating terrain evolution and new soil depths
*************************
Traceback (most recent call last):
  File "/Users/cmbarton/Library/GRASS/7.9/Modules/scripts/r.
landscape.evol", line 878, in <module>
    main()
  File "/Users/cmbarton/Library/GRASS/7.9/Modules/scripts/r.
landscape.evol", line 302, in main
    landscapeEvol(x, (x + 1), prefx, statsout,
region1['nsres'], masterlist, f)
  File "/Users/cmbarton/Library/GRASS/7.9/Modules/scripts/r.
landscape.evol", line 689, in landscapeEvol
    sdc.stdin.write('\n'.join(sdcolors))
TypeError: a bytes-like object is required, not 'str'

There also is a new Mac problem in that svn no longer comes with Apple Command Line Tools 11.5. This caused the the g.extension to fail, as it apparently still depends on svn to download code.

I was able to get it by downgrading to v. 11.4, but it will be a problem for Mac users in the future.

neteler commented 4 years ago

(please report the SVN-g.extension issue here: https://github.com/OSGeo/grass/issues)

neteler commented 4 years ago

Anything still to do here?

isaacullah commented 4 years ago

Hi Markus, apologies. Checking this out is still on my to do list, but pandemic lockdown, working from home, and balancing child care have really made it nearly impossible to do the "normal" amount of this kind of work I can do. I've finally managed to clear some things off my plate, however, so I still hope to get this done soon.

Sent from my📱

On Fri, Sep 18, 2020, 2:02 AM Markus Neteler notifications@github.com wrote:

Anything still to do here?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/OSGeo/grass-addons/issues/156#issuecomment-694748937, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACDYS5GTLKAPMVW3566GVFLSGMO27ANCNFSM4MXLHEHQ .