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

grass.jupyter: add SeriesMap and tests #3036

Merged
merged 26 commits into from
Nov 30, 2023
Merged

Conversation

chaedri
Copy link
Contributor

@chaedri chaedri commented Jun 6, 2023

SeriesMap creates a ipywidgets Slider for scrolling between a list of rasters and/or vectors.

series = gj.SeriesMap(height = 500)
series.add_rasters(["elevation_shade", "geology", "soils"])
series.add_vectors(["streams", "streets", "viewpoints"], color="blue")
series.d_barscale()
series.show()

A re-write of #2996 that is better: shorter code, a fewer bugs/ways to break it.

@chaedri chaedri changed the title SeriesMap redo grass.jupyter: add SeriesMap (better) Jun 6, 2023
@chaedri chaedri requested a review from petrasovaa June 6, 2023 15:22
@chaedri chaedri added the enhancement New feature or request label Jun 6, 2023
@chaedri chaedri changed the title grass.jupyter: add SeriesMap (better) grass.jupyter: add SeriesMap and tests Jun 6, 2023
Copy link
Contributor

@petrasovaa petrasovaa left a comment

Choose a reason for hiding this comment

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

This looks good! Just the test and flake8 are failing and I added couple comments.

python/grass/jupyter/region.py Outdated Show resolved Hide resolved
python/grass/jupyter/seriesmap.py Show resolved Hide resolved
@neteler neteler added this to the 8.4.0 milestone Aug 16, 2023
@chaedri
Copy link
Contributor Author

chaedri commented Sep 14, 2023

@wenzeslaus I'm having trouble getting the pylint disable=duplicate-code to work with the checks in GitHub. The way I have it setup now effectively removes the error on my local machine but it's still failing during the GitHub checks.

Copy link
Contributor

@petrasovaa petrasovaa left a comment

Choose a reason for hiding this comment

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

I think this is ready to be merged.

Copy link
Member

@wenzeslaus wenzeslaus left a comment

Choose a reason for hiding this comment

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

Please create an issue for the duplicate code. One issue just for the specific code issue is enough. The Pylint config is part of a bigger issue of reducing Pylint warnings.

python/grass/jupyter/utils.py Outdated Show resolved Hide resolved
python/grass/jupyter/tests/seriesmap_test.py Outdated Show resolved Hide resolved
@landam landam added the Python Related code is in Python label Nov 19, 2023
@chaedri
Copy link
Contributor Author

chaedri commented Nov 21, 2023

The only check that isn't passing is pytest. It passes on my local ubuntu machine but fails in GitHub actions with the error:

FAILED python/grass/jupyter/tests/seriesmap_test.py::test_default_init - NameError: Could not find a raster named precipitation_1
FAILED python/grass/jupyter/tests/seriesmap_test.py::test_render_layers - NameError: Could not find a raster named precipitation_3
FAILED python/grass/jupyter/tests/seriesmap_test.py::test_save - NameError: Could not find a raster named precipitation_1
================== 3 failed, 100 passed in 215.91s (0:03:35) ===================

I'm not sure why it would find these rasters on my machine but not in the checks because the rasters are created as a pytest Fixture in conftest.py by space_time_raster_dataset. Any ideas?

@petrasovaa
Copy link
Contributor

There is the same error for vectors...

Copy link
Contributor

@petrasovaa petrasovaa left a comment

Choose a reason for hiding this comment

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

There is still the bug in add_vectors and see Vashek's comment to create an issue about the code duplication.

@petrasovaa petrasovaa merged commit 731cab3 into OSGeo:main Nov 30, 2023
18 checks passed
@wenzeslaus wenzeslaus removed their request for review January 8, 2024 15:24
HuidaeCho pushed a commit to HuidaeCho/grass that referenced this pull request Jan 9, 2024
SeriesMap creates a ipywidgets Slider for scrolling between a list of rasters and/or vectors.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Python Related code is in Python
Development

Successfully merging this pull request may close these issues.

6 participants