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] search function for estimator overview not working #6105

Merged
merged 1 commit into from
Mar 23, 2024

Conversation

duydl
Copy link
Contributor

@duydl duydl commented Mar 11, 2024

Reference Issues/PRs

Fixes #5495

What does this implement/fix? Explain your changes.

I identified the location of the Js search function. It was written with jQuery which I was not familiar with. I just rewrote it in vanilla Js and the problem was fixed.

search

This is very basic so it may not but the most right or efficient. I am going to look further into the documentation and implement some other enhancements for the estimator overview in particular and the docs in general. Then I may find out why the jQuery didn't work or draw up some other solutions.

fkiraly
fkiraly previously approved these changes Mar 11, 2024
Copy link
Collaborator

@fkiraly fkiraly left a comment

Choose a reason for hiding this comment

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

NICE!

@fkiraly fkiraly added documentation Documentation & tutorials bugfix Fixes a known bug or removes unintended behavior labels Mar 11, 2024
@yarnabrina
Copy link
Collaborator

This is great. May be you want to add yourself as a contributor?

Just for my curiosity, did you figure out why was it working before v0.20.1?

@duydl
Copy link
Contributor Author

duydl commented Mar 11, 2024

I am working on an interactive table that could sort, group, display comprehensive tags info of estimators etc. Related to this issue: #1930

I extracted the info from all_tags and all_estimators to a json file and working on the dynamic table. You could check the progress here. And suggestions on how the info should be displayed is needed.
https://duydl.github.io/test/sktime-estimators-overview/estimator_overview_db.json
https://duydl.github.io/test/sktime-estimators-overview/

@fkiraly
Copy link
Collaborator

fkiraly commented Mar 12, 2024

This is great. May be you want to add yourself as a contributor?

Yes, of course! doc and maintenance, I suppose?

@fkiraly
Copy link
Collaborator

fkiraly commented Mar 12, 2024

I am working on an interactive table that could sort, group, display comprehensive tags info of estimators etc.

Nice!

What would be really interesting is to either give the user an option to select the tags they want displayed, and/or to predefine "interesting" tags if estimator types are selected, e.g., forecaster specific tags for forecaster.

Not all tags will be interesting to the user, mostly the ones that start with capability (there are some exception of older tags which do not conform to the naming convention, e.g., handles-missing-data, which really should be capability:missing_data)

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@fkiraly
Copy link
Collaborator

fkiraly commented Mar 12, 2024

(reopening until we've consolidated the various nice changes in multiple PR or otherwise)

@fkiraly fkiraly reopened this Mar 12, 2024
@duydl duydl changed the title fix: search function for estimator overview not working [BUG] search function for estimator overview not working Mar 13, 2024
@duydl
Copy link
Contributor Author

duydl commented Mar 13, 2024

Just for my curiosity, did you figure out why was it working before v0.20.1?

So it worked before :). I was curious too.
There is Uncaught ReferenceError: $ is not defined on developer tool when attempting to search in latest docs but not in v0.20.0 docs. It turns out jquery.js was no longer included in the recent versions.

When I searched for jquery in tagv0.20 I could not find any text or file except one instance being jquery.js -linguist-vendored in .gitattributes.

So the conf.py was changed somehow for jquery to be excluded or perhaps readthedocs/sphinx used to inject jQuery implicitly, but now no longer.

edited: the cause pyda_sphinx_theme dropping jquery pydata/pydata-sphinx-theme#1042

Copy link
Collaborator

@fkiraly fkiraly left a comment

Choose a reason for hiding this comment

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

This works, so I'll just merge it!

@fkiraly fkiraly merged commit 2bc1867 into sktime:main Mar 23, 2024
1 check passed
fkiraly pushed a commit that referenced this pull request Jun 7, 2024
…arch (#6147)

#### Reference Issues/PRs

#6105 #6106 #4905

The additional build on RTD can be found here in case it is not yet
rendered on sktime own RTD.
https://duydlsktime.readthedocs.io/en/latest/estimator_overview.html

#### What does this implement/fix? Explain your changes.
<!--
A clear and concise description of what you have implemented.
-->

An enhancement to estimator overview table. Support filtering by
estimator type and displaying tag.

#### Does your contribution introduce a new dependency? If yes, which
one?

It is implemented in vanilla js.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Fixes a known bug or removes unintended behavior documentation Documentation & tutorials
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] search functionality on estimator overview page not working
3 participants