-
-
Notifications
You must be signed in to change notification settings - Fork 284
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
GUI: call G_gisinit when forms are run standalone (e.g. from parser) #2489
Conversation
Repeated calls to G_gisinit are safe; When forms.py are loaded as a part of other code, one can assume that G_gisinit has already been called. See OSGeo#2484 for discussion.
gui/wxpython/gui_core/forms.py
Outdated
@@ -79,6 +79,9 @@ | |||
|
|||
# needed when started from command line and for testing | |||
if __name__ == "__main__": | |||
from grass.lib.gis import G_gisinit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The import here is too soon. The other GRASS-related imports are only later. (This needs a fix with GRASS Python libs just being installed in the system and rest of the code can just assume they are there, but the current situation is that the code can't assume that.)
All ctypes imports need to be in try-except ImportError
block as e.g. in gui/wxpython/nviz/wxnviz.py
. The idea is that the GUI components start and work even if ctypes are broken for some reason. Only the parts which need to work can fail, ideally gracefully. (Standalone NVIZ does not have to work, but main GUI should.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made a better version – import is done only before use. As G_gisinit("") is safe to call after initialization has been already done, calling it one more time wouldn't harm.
When a separate C module for signature file handling will be made, this code will have to change. In the meantime – it is better than failing GUI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great. You could also do that in _append_mapset_signatures
because after calling that function, items
can be already empty anyway. Interfacing with C would be then limited to _append_mapset_signatures
only.
Is it better now? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for addressing the review! I merged the main branch just to run the test with latest code. (The last run was old and failed for unrelated reason.)
…SGeo#2489) * GUI: call G_gisinit when forms are run standalone (e.g. from parser) Repeated calls to G_gisinit are safe; When forms.py are loaded as a part of other code, one can assume that G_gisinit has already been called. See OSGeo#2484 for discussion.
…SGeo#2489) * GUI: call G_gisinit when forms are run standalone (e.g. from parser) Repeated calls to G_gisinit are safe; When forms.py are loaded as a part of other code, one can assume that G_gisinit has already been called. See OSGeo#2484 for discussion.
Repeated calls to G_gisinit are safe; When forms.py are loaded as a part of other code, one can assume that
G_gisinit has already been (or will be) called.
See #2484 for discussion.