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

[Bug] GLWindow.ZoomToMap() is missing positional argument 'layers' #4878

Open
echoix opened this issue Dec 24, 2024 · 5 comments · May be fixed by #4899
Open

[Bug] GLWindow.ZoomToMap() is missing positional argument 'layers' #4878

echoix opened this issue Dec 24, 2024 · 5 comments · May be fixed by #4899
Labels
bug Something isn't working

Comments

@echoix
Copy link
Member

echoix commented Dec 24, 2024

Describe the bug

Error occurs when clicking on a first (vector) layer from the data tab. No problem right clicking a layer in the layers box and clicking Zoom to layer.

To reproduce

  1. Change 2D view to 3D view in the Map Display
  2. Double click on a layer from the data tab
    image
  3. See error in console tab

Expected behavior

Screenshots

image

System description

  • Operating System: Windows 11 24H226100.2605

  • GRASS GIS version: 8.5.0dev, from osgeo4w grass-dev 429 grass-dev-8.5-429-d6dec75dd-1 for commit d6dec75
    Originally found on Ubuntu 22.04 through WSL on the same windows computer, compiled from source, python 3.12.7, wxPython 4.2.2 too.

  • details about further software components

    • run g.version -rge in a GRASS GIS terminal session or check in
      the GUI menu "Help > About"
    • run python3 -c "import sys, wx; print(sys.version); print(wx.version())"
      to print the Python and wxPython version numbers
(Mon Dec 23 19:11:37 2024) Command finished (1 sec)                             
System Info                                                                     
GRASS version: 8.5.0dev                                                         
Code revision: d6dec75dd                                                        
Build date: 2024-12-23                                                          
Build platform: x86_64-w64-mingw32                                              
GDAL: 3.9.3                                                                     
PROJ: 9.5.0                                                                     
GEOS: 3.13.0                                                                    
SQLite: 3.46.1                                                                  
Python: 3.12.8                                                                  
wxPython: 4.2.2                                                                 
Platform: Windows-11-10.0.26100-SP0 (OSGeo4W)    

Additional context

I have a solution, to make the layer argument optional, and it works here.

@echoix echoix added the bug Something isn't working label Dec 24, 2024
@rohannallamadge
Copy link
Contributor

Hi @echoix,
I am encountering the same issue when double-clicking on a layer in the Data tab after switching to the 3D view. It's like the layers are playing hide and seek with me! 😅
Here’s what happens:

  1. When I clicked on a layer from the Layers tab in the 2D view, I got the following:
    image

  2. After switching to the 3D view, I encountered this:
    image
    3.However, if I directly click on a layer in the 3D view, I get an error.
    image

@rohannallamadge
Copy link
Contributor

whether passing the layers as an argument to ZoomToMap() is the correct approach?

image

@petrasovaa
Copy link
Contributor

ZoomToMap in mapwin/buffered.py and nviz/mapwindow.py should probably have the same optional arguments.

@echoix
Copy link
Member Author

echoix commented Dec 31, 2024

I've already got it fixed, the current commit in a staging branch I have is echoix@91b3b25. This branch is only waiting for my last two PRs made during the holidays to be merged (#4880 and #4876, #4880 was already approved but lost it after addressing some review comments), as it was all done incrementally but not committed, and got committed afterwards, so it's all in some random orders. The PRs were made by grouping some commits together that made sense in another branch. I kept my staging branch rebased after each PR merged.
I can't really make that branch a PR, as it needs commits before and after the currently open PRs, I don't know if they can be rebased safely (probably, but didn't work just for that).

@echoix
Copy link
Member Author

echoix commented Dec 31, 2024

My fix prevents the error, but even after working a bit for it, I didn't get to have the real functionality implemented (using the layers argument, I think I understand why the comment says it is still unused so far)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants