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] grass.jupyter.TimeSeriesMap incorrect layer rendering #5304

Open
petrasovaa opened this issue Mar 5, 2025 · 7 comments · May be fixed by #5416
Open

[Bug] grass.jupyter.TimeSeriesMap incorrect layer rendering #5304

petrasovaa opened this issue Mar 5, 2025 · 7 comments · May be fixed by #5416
Labels
bug Something isn't working
Milestone

Comments

@petrasovaa
Copy link
Contributor

Describe the bug

TimeSeriesMap is not rendering the layers correctly.

To reproduce

In doc/examples/notebooks/temporal.ipynb this example results in incorrect rendering:

precip_map = gj.TimeSeriesMap(use_region=True)
precip_map.add_raster_series("precip_sum_2010")
precip_map.d_legend(color="black", at=(10, 40, 2, 6))  # Add legend
precip_map.d_vect(map="boundary_county", fill_color="none")
precip_map.d_barscale()
precip_map.show()

Image

Expected behavior

The boundary layer and scalebar should be visible.

System description

This is in main branch only.

Additional context

Probably caused by some changes during last year GSoC. SeriesMap which has the same base class as TimeSeriesMap is working fine.

@petrasovaa petrasovaa added the bug Something isn't working label Mar 5, 2025
@petrasovaa petrasovaa added this to the 8.5.0 milestone Mar 5, 2025
@sudhanshu112233shukla
Copy link

I am close to solve this , can guide me so I can contribute..

@petrasovaa
Copy link
Contributor Author

I am close to solve this , can guide me so I can contribute..

Well, you need to understand the code in python/grass/jupyter/baseseriesmap.py and timeseriesmap.py to be able to solve this. I would start with trying to reproduce the problem. But if you are close to solving this, I am not sure what guidance specifically do you need?

@sudhanshu112233shukla
Copy link

@petrasovaa I only have two questions Should layer removal (to prevent stale layers) happen in TimeSeriesMap._render() or baseseriesmap.py’s _update_figure()? Are there existing functions to track/remove layers by timestep, or do I need to implement new logic.
Should TimeSeriesMap clear all layers between timesteps, or only those explicitly added via add_raster_series/add_vector_series? Is the root cause incomplete cleanup or incorrect rendering order.

@petrasovaa
Copy link
Contributor Author

@petrasovaa I only have two questions Should layer removal (to prevent stale layers) happen in TimeSeriesMap._render() or baseseriesmap.py’s _update_figure()? Are there existing functions to track/remove layers by timestep, or do I need to implement new logic. Should TimeSeriesMap clear all layers between timesteps, or only those explicitly added via add_raster_series/add_vector_series? Is the root cause incomplete cleanup or incorrect rendering order.

I don't know what is the root cause, I didn't investigate. What do you mean with stale layers? I don't see why you need to remove any layers.

@sudhanshu112233shukla
Copy link

@petrasovaa I think layer cleanup should happen in TimeSeriesMap._render() so we don’t accidentally break other parts of baseseriesmap.py. The goal is to only remove layers added by add_raster_series or add_vector_series (like temporary rainfall maps) while leaving static layers (e.g., a base map) untouched. The bug seems to happen because those temporary layers aren’t fully cleaned up between timesteps, not because of rendering order.

Here’s what I’ve tried: Reproduced the issue- When rendering timesteps, old layers stick around and overlap new ones.
Added a fix- I modified TimeSeriesMap to keep track of temporary layers and remove them before drawing new ones,
about to test it locally With the NC dataset, layers now refresh correctly without leftovers.
Does this approach make sense? or is there something I’ve missed?

@petrasovaa
Copy link
Contributor Author

Why don't you make a draft PR and I will look at it, without seeing the code I can't tell.

@sudhanshu112233shukla
Copy link

@petrasovaa Yes, I'll do it shortly.

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.

2 participants