Skip to content

Conversation

Shivansh-007
Copy link
Contributor

Relevant Issues

Closes #602

Description

Gets the fuzzy match of a search query, returns None if the ratio is below 90 else returns the fuzzy match. If the return is None, then we send our custom Error Message else it sends the link to the subsection of the WTF Python README.md.

Reasoning

Approved by a staff.

Screenshots

On Dark Theme

Screenshot from 2021-03-02 15-22-56

On Light Theme

Screenshot from 2021-03-02 15-22-43

(I went blind while teking it, and am helf blind rihgt now, so ignre ayn spellign misteke)

Did you:

Copy link
Member

@ChrisLovering ChrisLovering left a comment

Choose a reason for hiding this comment

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

Code looks good, just one issue while testing locally. I think it could be a good idea to have a .wtf list or similar, to output all of the available pages. Maybe one for after the paginator PR in merged.

@Shivansh-007
Copy link
Contributor Author

Code looks good, just one issue while testing locally. I think it could be a good idea to have a .wtf list or similar, to output all of the available pages. Maybe one for after the paginator PR in merged.

Alright will do.

@Xithrius Xithrius added area: backend Related to internal functionality and utilities season: evergreen status: needs review Author is waiting for someone to review and approve type: feature Relating to the functionality of the application. labels Mar 6, 2021
Copy link
Contributor

@ToxicKidz ToxicKidz left a comment

Choose a reason for hiding this comment

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

LGTM!

Base automatically changed from master to main March 13, 2021 20:09
@Shivansh-007 Shivansh-007 marked this pull request as draft March 14, 2021 13:02
Copy link
Member

@anand2312 anand2312 left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏻



class WTFPython(commands.Cog):
"""Cog that allows users to retrieve issues from GitHub."""
Copy link
Contributor

Choose a reason for hiding this comment

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

This docstring seems to be a leftover from copying the cog

Suggested change
"""Cog that allows users to retrieve issues from GitHub."""
"""Cog that allows geting WTF Python entries from the WTF Python repository."""

)

def fuzzy_match_header(self, query: str) -> Optional[str]:
"""Returns the fuzzy match of a query if its ratio is above 90 else returns None."""
Copy link
Contributor

Choose a reason for hiding this comment

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

Docstring mentions a ratio of 90, though the ratio is a global variable and currently 75

Suggested change
"""Returns the fuzzy match of a query if its ratio is above 90 else returns None."""
"""Returns the fuzzy match of a query if its ratio is above `MINIMUM_CERTAINTY` else returns None."""

@Shivansh-007 Shivansh-007 marked this pull request as draft May 11, 2021 05:07
Copy link
Contributor

@Kronifer Kronifer left a comment

Choose a reason for hiding this comment

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

Few nitpicks, but looks good!

@commands.command(aliases=("wtf",))
async def wtf_python(self, ctx: commands.Context, *, query: str) -> None:
"""
Search wtf python.
Copy link
Contributor

Choose a reason for hiding this comment

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

wtf is capitalised in other areas

@ToxicKidz ToxicKidz added status: waiting for author Waiting for author to address a review or respond to a comment and removed status: needs review Author is waiting for someone to review and approve labels May 17, 2021
@Shivansh-007
Copy link
Contributor Author

I don't think I am having enough time to look into this and it has been stale for quite a while now, so whoever wants can take over the PR, and deliver the feature!

@Xithrius Xithrius added up for grabs Available for anyone to work on and removed status: waiting for author Waiting for author to address a review or respond to a comment labels Aug 13, 2021
@Xithrius Xithrius removed their request for review August 16, 2021 02:28
Copy link
Member

@Xithrius Xithrius left a comment

Choose a reason for hiding this comment

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

@decorator-factory has noticed that this PR still uses fuzzywuzzy, which contains the GPLv2 license. We now use rapidfuzz because it uses the MIT license, so this new feature needs to reflect this change.

@brad90four
Copy link
Member

I think I can take this one, looks like rapidfuzz also has an extractOne method and the change from fuzzy_wuzzy to rapidfuzz is just the import statement.

@Xithrius
Copy link
Member

Xithrius commented Sep 3, 2021

@brad90four I shall assign you. rapidfuzz is currently what we're going with for the default replacement of fuzzy_wuzzy. Be sure to update the dependencies in poetry.lock.

@Xithrius Xithrius added status: WIP Work In Progress and removed up for grabs Available for anyone to work on labels Sep 3, 2021
@Xithrius
Copy link
Member

Xithrius commented Sep 5, 2021

Given that the restructure of this bot occurred, the main file of bot/exts/evergreen/wtf_python.py will need to be moved to bot/exts/utilities/wtf_python.py.

@Xithrius Xithrius added category: utilities Related to utilities category: fun Related to fun and games and removed category: evergreen category: fun Related to fun and games labels Sep 6, 2021
@brad90four
Copy link
Member

Should I get perms to Shivansh's repo in order to push changes to the feature branch or should I make a new PR with a branch from my own repo?

@Xithrius
Copy link
Member

Xithrius commented Sep 7, 2021

Make a new PR you will.

@brad90four brad90four mentioned this pull request Sep 7, 2021
4 tasks
@Xithrius Xithrius closed this Sep 7, 2021
@Shivansh-007 Shivansh-007 deleted the feature/wtf-python branch September 7, 2021 23:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: backend Related to internal functionality and utilities category: utilities Related to utilities status: WIP Work In Progress type: feature Relating to the functionality of the application.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Subsection Command for WTFPython
9 participants