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

add filter component and functionality #75

Merged
merged 4 commits into from
Jul 26, 2023

Conversation

emmalu
Copy link
Collaborator

@emmalu emmalu commented Jul 25, 2023

Summary:

  • What did I do?
    • Enabled the STAC catalog dropdown to allow possibility for visualizing data from multiple catalogs
    • Identified which collections in a catalog have assets, using that as a qualifier to determine if those collections have COGs that can be displayed (based on current - semi-limited lib functionality)
    • Added checkbox to control whether STAC collections in dropdown should be filtered (or not)
    • Consolidated code to set/refresh available collection items (in dropdown)
  • Why *** did I do it? (What was the motivation?)
    • This was a (June 23) UWG request, to enhance the UI experience
  • How *** can someone test or demo my changes?
    • Open the STAC widget and toggle the collections filter checkbox
Screenshot 2023-07-25 at 9 37 02 AM
STAC_v7_STAC_Filtered.mov

Fixes or Addresses Issue #: #66

Checklist before requesting a review:

  • My code follows the guidelines of this project
  • I performed a self-review of my code changes
  • I have completed spell-checking, removed unnecessary print statements & commented-out code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • New and existing unit tests pass locally with my changes

@emmalu emmalu requested a review from sandrahoang686 July 25, 2023 15:58
stac_ipyleaflet/stac_discovery/stac.py Show resolved Hide resolved
stac_ipyleaflet/stac_discovery/stac_widget.py Outdated Show resolved Hide resolved
stac_browser_url = self.stac_data["collection"]["href"].replace("https://", "https://stac-browser.maap-project.org/external/")
stac_browser_url = self.stac_data["collection"]["href"].replace(
"https://", "https://stac-browser.maap-project.org/external/"
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

💭 : I'm realizing that alot of these changes are probably due to the black formatter lol. But i'm thinking https://stac-browser.maap-project.org/external/ is a good candidate as a constant

@sandrahoang686
Copy link
Collaborator

Nice job on getting this quickly done 🙌🏼

💭 🧪 : I'm wondering when do we want to start adding tests with our changes in PRs? This change looks like it would be a good candidate to unit test but we dont have testing set up right now. I'm worried we would keep adding code and functionality creating a longer list of things to test in the future. Should we just start adding tests to our PR or we can come together and think of how to start implementing tests real soon!

@emmalu
Copy link
Collaborator Author

emmalu commented Jul 26, 2023

Nice job on getting this quickly done 🙌🏼

💭 🧪 : I'm wondering when do we want to start adding tests with our changes in PRs? This change looks like it would be a good candidate to unit test but we dont have testing set up right now. I'm worried we would keep adding code and functionality creating a longer list of things to test in the future. Should we just start adding tests to our PR or we can come together and think of how to start implementing tests real soon!

I think you're right - perhaps we should start integrating tests before the backlog of testing increases further. I do think it was important to make good progress on a feature in the interest of the overall sprint, so perhaps we can pause on implementing other features, and continue our focus to tidy-up: including testing, and perhaps some of the UI improvements we'll talk about later today.

@emmalu emmalu requested a review from sandrahoang686 July 26, 2023 13:56
Copy link
Collaborator

@sandrahoang686 sandrahoang686 left a comment

Choose a reason for hiding this comment

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

lgtm

@emmalu emmalu merged commit ccdb8b8 into main Jul 26, 2023
@emmalu emmalu self-assigned this Jul 28, 2023
@wildintellect wildintellect mentioned this pull request Aug 4, 2023
2 tasks
@wildintellect wildintellect deleted the feature/filter-compatible-collections branch September 27, 2023 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants