Skip to content
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

r.learn.ml2 fix import of rlearnlib #367

Merged
merged 4 commits into from
Dec 17, 2020

Conversation

anikaweinmann
Copy link
Contributor

@anikaweinmann anikaweinmann commented Dec 16, 2020

There were changed in the Makefile which leads to ModuleNotFoundError, because the lib path changed:

r.learn.train --help
Traceback (most recent call last):
  File "/home/mneteler/software/grass_master/dist.x86_64-pc-linux-gnu/scripts/r.learn.train", line 350, in <module>
    from rlearnlib.utils import (
ModuleNotFoundError: No module named 'rlearnlib'

These changes fix this problem.
(Developed with @griembauer)

@anikaweinmann anikaweinmann changed the title r.learn.ml2 fix import of rlearnlib WIP: r.learn.ml2 fix import of rlearnlib Dec 16, 2020
@anikaweinmann anikaweinmann changed the title WIP: r.learn.ml2 fix import of rlearnlib r.learn.ml2 fix import of rlearnlib Dec 16, 2020
@tmszi
Copy link
Member

tmszi commented Dec 16, 2020

There were changed in the Makefile which leads to ModuleNotFoundError, because the lib path changed:

r.learn.train --help
Traceback (most recent call last):
  File "/home/mneteler/software/grass_master/dist.x86_64-pc-linux-gnu/scripts/r.learn.train", line 350, in <module>
    from rlearnlib.utils import (
ModuleNotFoundError: No module named 'rlearnlib'

These changes fix this problem.
(Developed with @griembauer)

Thank you very much for reporting/testing this and your time. In my previous PR commit I made a change so that the files from the directory rlearnlib were installed again in the directory rlearnlib (in my case .grass7/addons/etc/r.learn.ml2/rlearnlib/) for consistency across Add-ons installation e.g. same as at wx.metadata Add-On (mdlib dir).

tomas@gentoo-gnu-linux:~$ ls .grass7/addons/etc/r.learn.ml2/rlearnlib/
indexing.py  __pycache__  raster.py  stats.py  transformers.py  utils.py
tomas@gentoo-gnu-linux:~$

Looks like I forgot to include this change, and this patch patch.zip (one line) will fix it.

install:
$(MKDIR) $(INST_DIR)/etc/r.learn.ml2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one line fix this error (previous changes isn't needed):
$(MKDIR) $(INST_DIR)/etc/r.learn.ml2/rlearnlib

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed all other changes. But I do not need the -p in the Makefile, because $(MKDIR) is in my system mkdir -p and I am not sure if the -p works in other systems e.g. Windows.

I have:
Distributor ID: LinuxMint
Description: Linux Mint 19.2 Tina
Release: 19.2
Codename: tina

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed all other changes. But I do not need the -p in the Makefile, because $(MKDIR) is in my system mkdir -p and I > am not sure if the -p works in other systems e.g. Windows.

Yes you are right. According $(MKDIR) variable definition in the file Platform.make.in, mkdir command has -p flag and then no needed extra flag definition in this Makefile.

@tmszi
Copy link
Member

tmszi commented Dec 17, 2020

Could @anikaweinmann test it and confirm that it works, please?

@anikaweinmann
Copy link
Contributor Author

@tmszi yes it is working for me. But I'm not sure that the $(MKDIR) -p is working for every system.
On my VirtualBox with Windows I get:
image

@hellik
Copy link
Member

hellik commented Dec 17, 2020

@tmszi
Copy link
Member

tmszi commented Dec 17, 2020

@tmszi yes it is working for me. But I'm not sure that the $(MKDIR) -p is working for every system.
On my VirtualBox with Windows I get:
image

Thanks for info. But Add-On installation work differently on MS Windows (precompiled packages are installed on the client, Add-On compilation takes place on the server which use OSGeo4W installer) e.g. wx.metadata Add-On use $(MKDIR) -p too, and compilation is alright according https://wingrass.fsv.cvut.cz/grass78/x86_64/addons/latest/logs/. The installation wx.metadata Add-On on MS Windows was not successful, but error not related with $(MKDIR) -p.

wx.metadata Add-On installation dir structure (mdlib 'same' as rlearnlib) and seems alright:

wx_metadata_add_on_structure

Copy link
Contributor Author

@anikaweinmann anikaweinmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tmszi yes it is working for me. But I'm not sure that the $(MKDIR) -p is working for every system.
On my VirtualBox with Windows I get:
image

Thanks for info. But Add-On installation work differently on MS Windows (precompiled packages are installed on the client, Add-On compilation takes place on the server which use OSGeo4W installer) e.g. wx.metadata Add-On use $(MKDIR) -p too, and compilation is alright according https://wingrass.fsv.cvut.cz/grass78/x86_64/addons/latest/logs/. The installation wx.metadata Add-On on MS Windows was not successful, but error not related with $(MKDIR) -p.

wx.metadata Add-On installation dir structure (mdlib 'same' as rlearnlib) and seems alright:

wx_metadata_add_on_structure

Okay. Then is your change fine for me.

install:
$(MKDIR) $(INST_DIR)/etc/r.learn.ml2
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed all other changes. But I do not need the -p in the Makefile, because $(MKDIR) is in my system mkdir -p and I am not sure if the -p works in other systems e.g. Windows.

I have:
Distributor ID: LinuxMint
Description: Linux Mint 19.2 Tina
Release: 19.2
Codename: tina

@tmszi
Copy link
Member

tmszi commented Dec 17, 2020

@tmszi yes it is working for me. But I'm not sure that the $(MKDIR) -p is working for every system.

I try install r.learn.ml2 on MS Windows, and I get error:

C:\Users\IEUser>g.extension r.learn.ml2
Downloading precompiled GRASS Addons <r.learn.ml2>...
Fetching <r.learn.ml2> from
<http://wingrass.fsv.cvut.cz/grass79/x86_64/addons/grass-7.9.dev/r.learn.ml2.zip>
(be patient)...
Updating extensions metadata file...
Updating extension modules metadata file...
WARNING: No metadata available for module 'transformers'.
WARNING: No metadata available for module 'stats'.
WARNING: No metadata available for module 'r.learn.predict'.
WARNING: No metadata available for module 'indexing'.
WARNING: No metadata available for module 'r.learn.train'.
WARNING: No metadata available for module 'raster'.
WARNING: No metadata available for module 'utils'.
ERROR: Unable to read manual page: [Errno 2] No such file or directory:
       'C:\\Users\\IEUser\\AppData\\Roaming\\GRASS7\\addons\\docs\\html\\transformers.html'

r.learn.ml2 Add-On installation dir structure (rlearnlib) seems alright:

r_learn_ml2_add_on_structure

Copy link
Member

@tmszi tmszi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You missed the creation of 'rlearnlib' dir in the Makefile, check my commit.

@anikaweinmann anikaweinmann merged commit e8ce288 into OSGeo:master Dec 17, 2020
@anikaweinmann anikaweinmann deleted the r.learn.ml2_importlibfix branch December 17, 2020 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants