grass.pygrass: Lazy load list of commands for module shortcuts

[wenzeslaus]

Importing grass.pygrass.modules required the list to be loaded because the shortcuts module is loaded by the modules module. Now the list is loaded only when the dir function is called for the first time.

This is using the technique from grass init file for lazy loading translations.

This allows grass.pygrass.modules to be imported without an active session, so this simplifies the GridModule tests (and its imports in general).

[echoix]

Seems ok, but I already had a problem using function attributes with type checking.

It seems that type checking didn't support function attributes at all point in time. When I encountered the problem, I wasn't able to find a pattern that respected all our supported versions.

It was difficult to understand at first what that pattern was. It is assigning a value to an object, but that object is a function, a bit counterintuitive.

Is there a difference (like performance or number of evaluations/assignments) between that or simply using a variable? Is it like a static variable?

Other than that, this and the couple PRs you just filed (didn't finish reading yet), seem to be in the right direction to allow me to run gunittests with pytest, and also removing unneeded interdependencies (when grass is still being built)

[wenzeslaus]

Seems ok, but I already had a problem using function attributes with type checking.

It seems that type checking didn't support function attributes at all point in time. When I encountered the problem, I wasn't able to find a pattern that respected all our supported versions.

It was difficult to understand at first what that pattern was. It is assigning a value to an object, but that object is a function, a bit counterintuitive.

Discussions around PEP 232 – Function Attributes are full of "functions in Python are objects". Objects have attributes, e.g.:

>>> import math
>>> math.sqrt.__doc__
'Return the square root of x.'

Still, I agree that this is low-level stuff not obvious to most readers.

Is there a difference (like performance or number of evaluations/assignments) between that or simply using a variable?

Performance? No idea. Probably none.

The difference is that there is an explicit association of the variable with the function, i.e., it says don't use it outside of the context of function. Thinking about it again, the attributes themselves should probably be private/protected to express clearly that there the intention is not to use it outside of the function, but the initial assignment would break that rule, so I don't know.

Is it like a static variable?

Yes, that's what you get for searching "static variable in Python". This of course assumes C knowledge, so not straightforward.

The case in this PR can be turned into a classic function result cashing problem. I see there are some functions in functools for that.

Other than that, this and the couple PRs you just filed (didn't finish reading yet), seem to be in the right direction to allow me to run gunittests with pytest, and also removing unneeded interdependencies (when grass is still being built)

Let me know if you find issues like these. If there are some left in Python, we have functions to fix them.

[echoix]

It seems that type checking didn't support function attributes at all point in time. When I encountered the problem, I wasn't able to find a pattern that respected all our supported versions.

I thought I had linked the issue I was referring to. See the end of that old, long, issue python/mypy#2087.

Yes, that's what you get for searching "static variable in Python". This of course assumes C knowledge, so not straightforward.

Was it even python 3 back then?

Other than that, this and the couple PRs you just filed (didn't finish reading yet), seem to be in the right direction to allow me to run gunittests with pytest, and also removing unneeded interdependencies (when grass is still being built)

Let me know if you find issues like these. If there are some left in Python, we have functions to fix them.

Are you talking about the missing unittest-via-pytest roadblocks, or special uses of function attributes that make typing really hard/impossible?
If it is about issues/roadblocks, I'll try to surface them out next time I dive into that subject, still in my queue of personal goals blocked by other side quests.

Discussions around PEP 232 – Function Attributes are full of "functions in Python are objects". Objects have attributes, e.g.:

>>> import math
>>> math.sqrt.__doc__
'Return the square root of x.'

Still, I agree that this is low-level stuff not obvious to most readers.

__doc__ and others are kind of special cases, as they are expected to maybe exist. Here (and my problem I was referring to, it was an root init file (like the real grass one or in gunittest, I don't remember).

[echoix]

One type of the errors remaining is errors that make the files not importable at all if not everything is not installed and run in a specific way.
To quickly see what they could be, we need to widen the search of pytest in pyproject.toml (approaching the defaults of pytest)

grass/pyproject.toml

Lines 12 to 23 in 4ecb43e

	[tool.pytest.ini_options]
	minversion = "6.0"
	python_files = "*/tests/*_test.py"
	addopts = """
	--ignore-glob='dist.*'
	--ignore-glob='bin.*'
	--ignore-glob='*/tests/data/*'
	--ignore-glob='*/grass/pygrass/tests/*'
	--doctest-glob='*doctest*.txt'
	--ignore='raster/r.category/test_rcategory_doctest.txt'
	"""
	timeout = 300

For example, removing python_files and using the defaults

[tool.pytest.ini_options]
minversion = "6.0"
# python_files = "*/tests/*_test.py"
addopts = """
    --ignore-glob='dist.*'
    --ignore-glob='bin.*'
    --ignore-glob='*/tests/data/*'
    --ignore-glob='*/grass/pygrass/tests/*'
    --doctest-glob='*doctest*.txt'
    --ignore='raster/r.category/test_rcategory_doctest.txt'
"""
timeout = 300

or

[tool.pytest.ini_options]
minversion = "6.0"
python_files = "*/tests/*_test.py" "*/testsuite/*.py"
addopts = """
    --ignore-glob='dist.*'
    --ignore-glob='bin.*'
    --ignore-glob='*/tests/data/*'
    --ignore-glob='*/grass/pygrass/tests/*'
    --doctest-glob='*doctest*.txt'
    --ignore='raster/r.category/test_rcategory_doctest.txt'
"""
timeout = 300

You might need to use the --continue-on-collection-errors option to pytest if you can't see more than one error.

[wenzeslaus]

One type of the errors remaining is errors that make the files not importable at all if not everything is not installed and run in a specific way.

Yes, these are the once I was asking about.

To quickly see what they could be, we need to widen the search of pytest in pyproject.toml (approaching the defaults of pytest)

This likely hits legacy test files which were possibly never intended to run automatically. They need to be fixed to run either in the pytest or the gunittest way. Right now, I'm looking for issues in the package as they would be critical to users (if there are any). I would like to chase the tests files later unless someone does that before me.

[wenzeslaus]

I thought I had linked the issue I was referring to. See the end of that old, long, issue python/mypy#2087.

I read only 2023 comments, but what a strange discussion. Function attributes are a language feature no one can use when they want to do typing. Okay, maybe typed functions should be only functions. I can live with that. I guess my issue now is that I need to go into callable class or functools to achieve the same.

Yes, that's what you get for searching "static variable in Python". This of course assumes C knowledge, so not straightforward.

Was it even python 3 back then?

No, it looks like early Python 2.

Are you talking about the missing unittest-via-pytest roadblocks, or special uses of function attributes that make typing really hard/impossible?

unittest-via-pytest roadblocks like "importing grass.bla.bla requires a session with NC SPM".

...Here (and my problem I was referring to, it was an root init file (like the real grass one or in gunittest, I don't remember).

The real grass/__init__.py one has the lazy loaded translate function done with the same function attribute technique this PR is using.

[echoix]

This likely hits legacy test files which were possibly never intended to run automatically. They need to be fixed to run either in the pytest or the gunittest way. Right now, I'm looking for issues in the package as they would be critical to users (if there are any). I would like to chase the tests files later unless someone does that before me.

I wasn't suggesting that the tests need to work, that's for later, but it can surface some problems that prevent simple imports of the files, a bit throughout the code. It's not exhaustive, but the collection errors from files outside the test folders are potential issues to solve.

[echoix]

As for the function attribute part, I think my concerns are answered, if there's something it'll be a (my) problem for later.

[wenzeslaus]

This will make certain pygrass imports faster. It would be good to have it in 8.4, but I would hate to break things close to the release. Can someone give this a second look? (Second review besides @echoix)

[neteler]

I have patched my local GRASS GIS version. Anything specific I can test?

[wenzeslaus]

I have patched my local GRASS GIS version. Anything specific I can test?

Oh, I was thinking someone should read the code, but an extra manual test would be nice too.

These two steps should work in any version:

grass --tmp-location XY --exec python

import grass.pygrass.modules

Then try this:

PYTHONPATH=$(grass --config python-path) python

import grass.pygrass.modules

The same Python code, but just PYTHONPATH setup fails with older versions, but works with main.

Further you can run:

from grass.pygrass.modules.shortcuts import general
dir(general)

This PR should not break that.

[neteler]

All good:

mneteler@caddy: ~$ software/grass_main/bin.x86_64-pc-linux-gnu/grass --tmp-location XY --exec python
Option --tmp-location is deprecated, use --tmp-project instead
Python 3.12.3 (main, Apr 17 2024, 00:00:00) [GCC 13.2.1 20240316 (Red Hat 13.2.1-7)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import grass.pygrass.modules
>>> 


mneteler@caddy: ~$ PYTHONPATH=$(software/grass_main/bin.x86_64-pc-linux-gnu/grass --config python-path) python
Python 3.12.3 (main, Apr 17 2024, 00:00:00) [GCC 13.2.1 20240316 (Red Hat 13.2.1-7)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import grass.pygrass.modules
>>> 
>>> 
>>> from grass.pygrass.modules.shortcuts import general
>>> dir(general)
['access', 'cairocomp', 'copy', 'dirseps', 'download_location', 'extension', 'extension_all', 'filename', 'findetc', 'findfile', 'gisenv', 'gui', 'gui_animation', 'gui_datacatalog', 'gui_dbmgr', 'gui_gcp', 'gui_gmodeler', 'gui_iclass', 'gui_image2target', 'gui_mapswipe', 'gui_photo2image', 'gui_psmap', 'gui_rdigit', 'gui_rlisetup', 'gui_timeline', 'gui_tplot', 'gui_vdigit', 'list', 'manual', 'mapset', 'mapsets', 'message', 'mkfontcap', 'parser', 'pnmcomp', 'ppmtopng', 'proj', 'region', 'remove', 'rename', 'search_modules', 'tempfile', 'version']
>>> 

PS: of course I enjoy to see "Option --tmp-location is deprecated, use --tmp-project instead" :-)

[echoix]

I updated the branch, lots of this have moved in the last 3 weeks.

[echoix]

Still ok with me