grass.pygrass: VisibleMapset: fix reading search path by petrasovaa · Pull Request #2584 · OSGeo/grass

petrasovaa · 2022-09-19T15:23:58Z

VisibleMapset class was not reading search path properly, instead it was always just writing PERMANENT only into the path.

tmszi

Tested and work as expected.

tmszi · 2022-09-19T18:35:16Z

It requires modifying the GridModule class tests.

tmszi · 2022-09-20T04:35:34Z

Complex test with 3c8d6b8:

SEARCH_PATH file doesn't exists (passed):

GRASS nc_spm_08_grass7/landsat:~ > file grassdata/nc_spm_08_grass7/landsat/SEARCH_PATH 
grassdata/nc_spm_08_grass7/landsat/SEARCH_PATH: cannot open `grassdata/nc_spm_08_grass7/landsat/SEARCH_PATH' (No such file or directory)

GRASS nc_spm_08_grass7/landsat:~ > g.mapsets -p
Accessible mapsets:
landsat PERMANENT

GRASS nc_spm_08_grass7/landsat:~ > cat /tmp/visible_mapset_test.py 
#!/usr/bin/env python3

import sys

from grass.pygrass.gis import VisibleMapset

def main():
    vm = VisibleMapset(mapset=sys.argv[1])
    print(vm.read())

if __name__ == '__main__':
    main()

GRASS nc_spm_08_grass7/landsat:~ > python /tmp/visible_mapset_test.py $(g.gisenv get="MAPSET")
['landsat', 'PERMANENT']

SEARCH_PATH file is empty (passed):

GRASS nc_spm_08_grass7/landsat:~ > file grassdata/nc_spm_08_grass7/landsat/SEARCH_PATH 
grassdata/nc_spm_08_grass7/landsat/SEARCH_PATH: empty

GRASS nc_spm_08_grass7/landsat:~ > g.mapsets -p
Accessible mapsets:
landsat

GRASS nc_spm_08_grass7/landsat:~ > python /tmp/visible_mapset_test.py $(g.gisenv get="MAPSET")
['landsat']

SEARCH_PATH file exist (passed, and a new bug appear with g.mapsets module):

GRASS nc_spm_08_grass7/landsat:~ > file grassdata/nc_spm_08_grass7/landsat/SEARCH_PATH 
grassdata/nc_spm_08_grass7/landsat/SEARCH_PATH: ASCII text

GRASS nc_spm_08_grass7/landsat:~ > cat grassdata/nc_spm_08_grass7/landsat/SEARCH_PATH 
landsat
PERMANENT
climate_2000_2012

GRASS nc_spm_08_grass7/landsat:~ > python /tmp/visible_mapset_test.py $(g.gisenv get="MAPSET")
['landsat', 'PERMANENT', 'climate_2000_2012', '']

It seems that with this last test example (Python code output) we have an extra unnecessary empty '' MAPSET name due the g.mapsets module writing an extra \n to the SEARCH_PATH file. PR #2586.

GRASS nc_spm_08_grass7/landsat:~ > cat -e grassdata/nc_spm_08_grass7/landsat/SEARCH_PATH 
landsat$
PERMANENT$
climate_2000_2012$
$

tmszi · 2022-09-20T05:39:25Z

python/grass/pygrass/gis/__init__.py

-        self._write(lns)
-        return lns
+        try:
+            with open(self.spath, "rb") as f:


open file func mode param arg should be "r" only according output bellow and we don't need decode readed lines:

GRASS nc_spm_08_grass7/landsat:~ > g.mapsets operation=set mapset=landsat GRASS nc_spm_08_grass7/landsat:~ > file grassdata/nc_spm_08_grass7/landsat/SEARCH_PATH grassdata/nc_spm_08_grass7/landsat/SEARCH_PATH: ASCII text

Same for _write() method, open file func with mode param arg should be w only.

def _write(self, mapsets): """Write to SEARCH_PATH file the changes in the search path :param list mapsets: a list of mapset's names """ with open(self.spath, "w") as f: f.write("\n".join([m for m in mapsets if m in self.location.mapsets()]))

extend() method without decode func

def extend(self, mapsets): """Add more mapsets to the search path :param list mapsets: a list of mapset's names """ final = self.read() final.extend( [ m for m in mapsets if m in self.location.mapsets() and m not in final ] ) self._write(final)

tmszi · 2022-09-21T18:46:39Z

python/grass/pygrass/gis/__init__.py

-            f.write(b"\n".join([encode(m) for m in mapsets if m in ms]))
+        with open(self.spath, "w") as f:
+            ms = self.location.mapsets()
+            f.write("\n".join([m for m in mapsets if m in ms]))


Small note (it is not bug):

g.mapsets module write \n on the EOF for every line in the SEARCH_PATH file:

GRASS nc_spm_08_grass7/landsat:~ > g.mapsets operation=set mapset=landsat GRASS nc_spm_08_grass7/landsat:~ > g.mapsets operation=add mapset=PERMANENT GRASS nc_spm_08_grass7/landsat:~ > cat -e grassdata/nc_spm_08_grass7/landsat/SEARCH_PATH landsat$ PERMANENT$

VisibleMapset class instance _write method do not write \n on the EOF on the last line in the SEARCH_PATH file:

GRASS nc_spm_08_grass7/landsat:~ > g.mapsets -l Available mapsets: PERMANENT climate_2000_2012 landsat tomas GRASS nc_spm_08_grass7/landsat:~ > cat /tmp/test.py #!/usr/bin/env python3 import sys from grass.pygrass.gis import VisibleMapset def main(): vm = VisibleMapset(mapset=sys.argv[1]) vm.extend(["climate_2000_2012", "tomas"]) print(vm.read()) if __name__ == '__main__': main() GRASS nc_spm_08_grass7/landsat:~ > python /tmp/test.py $(g.gisenv get="MAPSET") ['landsat', 'PERMANENT', 'climate_2000_2012', 'tomas'] GRASS nc_spm_08_grass7/landsat:~ > cat -e grassdata/nc_spm_08_grass7/landsat/SEARCH_PATH landsat$ PERMANENT$ climate_2000_2012$ tomasGRASS nc_spm_08_grass7/landsat:~ >

tomasGRASS nc_spm_08_grass7/landsat:~ > this line should be (missing $ which is \n)

tomas$ GRASS nc_spm_08_grass7/landsat:~ >

Fixed with (line 459):

eof = "\n" f.write(eof.join([m for m in mapsets if m in ms]) + eof)

and (line 469):

f.write("%s\n" % mapset)

petrasovaa · 2022-09-23T09:47:25Z

No I got confused, didn't you @tmszi change the behavior of g.mapsets in #2586 to avoid writing the newline at the end? Should I revert the last commit in this PR?

tmszi · 2022-09-23T18:14:28Z

No I got confused, didn't you @tmszi change the behavior of g.mapsets in #2586 to avoid writing the newline at the end? Should I revert the last commit in this PR?

With PR #2586 I solve problem with additional empty line ($ == \n) when g.mapsets module write to the SEARCH_PATH file.

GRASS nc_spm_08_grass7/landsat:~ > cat -e grassdata/nc_spm_08_grass7/landsat/SEARCH_PATH 
landsat$
PERMANENT$
climate_2000_2012$
$

should be

GRASS nc_spm_08_grass7/landsat:~ > cat -e grassdata/nc_spm_08_grass7/landsat/SEARCH_PATH 
landsat$
PERMANENT$
climate_2000_2012$

VisibleMapset class instance _write() method don't write \n on the last line ($ == \n) MAPSET -> MAPSET\n and your last commit a3e05df solve it and is needed:

Example when _write() method write to SEARCH_PATH file:

GRASS nc_spm_08_grass7/landsat:~ > cat -e grassdata/nc_spm_08_grass7/landsat/SEARCH_PATH 
landsat$
PERMANENT$
climate_2000_2012$
tomasGRASS nc_spm_08_grass7/landsat:~ >

should be

GRASS nc_spm_08_grass7/landsat:~ > cat -e grassdata/nc_spm_08_grass7/landsat/SEARCH_PATH 
landsat$
PERMANENT$
climate_2000_2012$
tomas$
GRASS nc_spm_08_grass7/landsat:~ >

this line tomasGRASS nc_spm_08_grass7/landsat:~ > should be (missing \n on the EOF tomas to tomas$)

tomas$
GRASS nc_spm_08_grass7/landsat:~ >

tmszi · 2022-10-04T04:00:19Z

Could we move forward with this PR, please?

* handle missing search path file, do not write since this is only reading * use text mode for writing/reading * write newline after each mapset

grass.pygrass: VisibleMapset: fix reading search path

4ef7a1f

petrasovaa added bug Something isn't working backport_needed Python Related code is in Python labels Sep 19, 2022

petrasovaa added this to the 8.2.1 milestone Sep 19, 2022

petrasovaa requested a review from tmszi September 19, 2022 15:23

petrasovaa mentioned this pull request Sep 19, 2022

grass.pygrass: Reset back mapset search path after GridModule class instance finish #2567

Merged

tmszi approved these changes Sep 19, 2022

View reviewed changes

handle missing search path file, do not write since this is only reading

3c8d6b8

tmszi reviewed Sep 20, 2022

View reviewed changes

use text mode for writing/reading

d7ba5e5

petrasovaa requested a review from tmszi September 21, 2022 08:28

tmszi approved these changes Sep 21, 2022

View reviewed changes

write newline after each mapset

a3e05df

petrasovaa merged commit a94a3af into OSGeo:main Oct 6, 2022

petrasovaa deleted the fix-pygrass-visible-mapset branch October 6, 2022 13:11

petrasovaa added a commit that referenced this pull request Oct 6, 2022

grass.pygrass: VisibleMapset: fix reading search path (#2584)

cac51cd

* handle missing search path file, do not write since this is only reading * use text mode for writing/reading * write newline after each mapset

petrasovaa removed the backport_needed label Oct 6, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Comments

grass.pygrass: VisibleMapset: fix reading search path#2584

grass.pygrass: VisibleMapset: fix reading search path#2584
petrasovaa merged 4 commits intoOSGeo:mainfrom
petrasovaa:fix-pygrass-visible-mapset

petrasovaa commented Sep 19, 2022

Uh oh!

tmszi left a comment

Uh oh!

tmszi commented Sep 19, 2022

Uh oh!

tmszi commented Sep 20, 2022 •

edited

Loading

Uh oh!

tmszi Sep 20, 2022 •

edited

Loading

Uh oh!

tmszi Sep 21, 2022

Uh oh!

petrasovaa commented Sep 23, 2022

Uh oh!

tmszi commented Sep 23, 2022 •

edited

Loading

Uh oh!

tmszi commented Oct 4, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Comments

Conversation

petrasovaa commented Sep 19, 2022

Uh oh!

tmszi left a comment

Choose a reason for hiding this comment

Uh oh!

tmszi commented Sep 19, 2022

Uh oh!

tmszi commented Sep 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tmszi Sep 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tmszi Sep 21, 2022

Choose a reason for hiding this comment

Uh oh!

petrasovaa commented Sep 23, 2022

Uh oh!

tmszi commented Sep 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tmszi commented Oct 4, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tmszi commented Sep 20, 2022 •

edited

Loading

tmszi Sep 20, 2022 •

edited

Loading

tmszi commented Sep 23, 2022 •

edited

Loading