-
Notifications
You must be signed in to change notification settings - Fork 1
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
Functionality catalog #328
Conversation
merge main
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ferdiko! Can you look into the Session
changes mentioned below? I think that we should avoid storing the session in QueryRep
to avoid coupling them. QueryRep
should just be a class that represents a query.
Changing that and the paths below should fix the tests as well.
Please also add back the script you used to extract the keywords for the geospatial queries (just commit it under |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good - I tested it out on my end and it seems to be fine. Thanks! I'll merge the PR.
Hey Geoff, I "unit tested" the functionality catalog but it would be great if you could check if it breaks any of your things.