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

[Feat] Reduce duplicate code between grass.jupyter.SeriesMap and grass.jupyter.TimeSeriesMap #3276

Closed
chaedri opened this issue Nov 29, 2023 · 5 comments · Fixed by #3468
Closed
Labels
enhancement New feature or request gsoc Reserved for Google Summer of Code student(s) Python Related code is in Python
Milestone

Comments

@chaedri
Copy link
Contributor

chaedri commented Nov 29, 2023

Is your feature request related to a problem? Please describe.
In #3036, we added a new visualization class for viewing and animating a series of related rasters or vectors. Similar to TimeSeriesMap, SeriesMap uses ipywidgets. Because of the similarities between the two classes, there is a significant amount of duplicated code, particularly in the class __init__ and image rendering.

Describe the solution you'd like
The code should be refactored to reduce duplication. One possible solution is a base class that consolidates methods used by both classes.

@chaedri chaedri added the enhancement New feature or request label Nov 29, 2023
@29riyasaxena
Copy link
Contributor

Hi @chaedri, I would like to work on this. I think it's a good idea to create a base class that both can share to avoid writing the same code twice. Here's my plan:

  1. Create a new base class (BaseMap) and move the common code into this class (from __init__ and _render_baselayers).
  2. Make TimeSeriesMap and SeriesMap inherit from BaseMap.
  3. Replace the common code in TimeSeriesMap and SeriesMap with calls to the base class code.

@petrasovaa
Copy link
Contributor

Hi @chaedri, I would like to work on this. I think it's a good idea to create a base class that both can share to avoid writing the same code twice. Here's my plan:

  1. Create a new base class (BaseMap) and move the common code into this class (from __init__ and _render_baselayers).
  2. Make TimeSeriesMap and SeriesMap inherit from BaseMap.
  3. Replace the common code in TimeSeriesMap and SeriesMap with calls to the base class code.

Welcome! Your plan sounds good, I would probably name it BaseSeriesMap. There are many methods that have very similar, but not identical code, so there may be a need to generalize the code a bit. I think you will find it helpful to first run the notebook with TimeSeriesMap to better understand how it is being used.

@chaedri
Copy link
Contributor Author

chaedri commented Mar 4, 2024

Welcome @29riyasaxena ! This sounds good to me too. I'll add the example mentioned in #3467 so you can see that too.

@29riyasaxena
Copy link
Contributor

Welcome @29riyasaxena ! This sounds good to me too. I'll add the example mentioned in #3467 so you can see that too.

Hi @chaedri, I've made adjustments to the code and have just pushed it to #3468. I'd appreciate it if you could take a look and provide your feedback. Thanks!

29riyasaxena added a commit to 29riyasaxena/grass that referenced this issue Mar 6, 2024
29riyasaxena added a commit to 29riyasaxena/grass that referenced this issue Mar 6, 2024
@Jasmitha-Mannam
Copy link

Hi..... ,I would like to work on this problem statement as a part of GSOC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request gsoc Reserved for Google Summer of Code student(s) Python Related code is in Python
Projects
None yet
4 participants