Pride Leader(pl) by Anubhav1603 · Pull Request #485 · python-discord/sir-lancebot

Anubhav1603 · 2020-10-06T12:35:50Z

Relevant Issues

Closes #208

Description

command can be invoked with pride_leader name or without name, if name is not provided then bot randomly picks name from json.

Screenshots

Did you:

Join the Python Discord Community?
If dependencies have been added or updated, run pipenv lock?
Lint your code (pipenv run lint)?
Set the PR to allow edits from contributors?

tagptroll1

I'll start out with these, probably going to need to do a 2nd take as the comments clutter up the view a bit for me.

tagptroll1 · 2020-10-06T13:55:34Z

+
+
+class PrideLeader(commands.Cog):
+    """Gives a Pride Leader Info."""


Suggested change

"""Gives a Pride Leader Info."""

"""Gives information about Pride Leaders."""

It's giving information about multiple leaders, not one. Maybe also a different name for "Pride Leaders", I'm unsure about what that means. Sounds like a cult, or regime in my opinion. "People of status in the Pride / LGBTQ Community" ?

tagptroll1 · 2020-10-06T14:05:05Z

+        return embed
+
+    @commands.command(name="prideleader", aliases=['pl'])
+    async def pl(self, ctx: commands.Context, *, pride_leader_name: str = None) -> None:


Write out the function names.. please. It's still a function that needs to have a descriptive name. Line break it if it gets too long
You can also directly import Context and type ctx with that if you need more space.

Additionally, Instead of defaulting pride_leader_name to None, remove the default, and add an error handler for the command.

tagptroll1 · 2020-10-06T14:05:16Z

+        if pride_leader_name is None:
+            log.trace("Name not provided by the user so selecting random from json.")
+            name_list = [name for name in self.pride.keys()]
+            pride_leader_name = random.choice(name_list)


Move to an error handler instead

That way we can change it up if we don't want to randomize the result, and separates the logic

I wnt to randomize for a reason so people can know about different LGBTQ+ leader

tagptroll1 · 2020-10-06T14:06:40Z

+
+    @commands.command(name="prideleader", aliases=['pl'])
+    async def pl(self, ctx: commands.Context, *, pride_leader_name: str = None) -> None:
+        """Provides info about pride leader randomly without taking any args or by taking name."""


This reads weird

tagptroll1 · 2020-10-06T14:07:00Z

+
+
+def setup(bot: commands.Bot) -> None:
+    """Cog loader for drag queen name generator."""


tagptroll1 · 2020-10-06T14:08:42Z

+        self.pride = self.load_json()
+
+    @staticmethod
+    def load_json() -> dict:
+        """Loads pride leader information from static json resource."""
+        explanation_file = Path("bot/resources/pride/prideleader.json")
+        with explanation_file.open(encoding="utf8") as json_data:
+            pride = json.load(json_data)
+
+        return pride


I didn't see the method being used anywhere else, just do it all in the init. "Load_json" insn't really a descriptive name either. It could be any json

its used in embed builder

Co-authored-by: Thomas Petersson <Thomas@petersson.priv.no>

Co-authored-by: Rohan Reddy Alleti <roalleti@gmail.com>

Co-authored-by: Rohan Reddy Alleti <rohanjnr44@gmail.com>

RohanJnr · 2020-10-15T10:33:36Z

+        if leader is None:
+            log.trace("Got invalid name.")
+            final_embed = self.invalid_embed_generate(pride_leader_name)


This check should be inside the previous else block as that is where you are validating the given name.
example:

@commands.command(name="prideleader", aliases=['pl']) async def pride_leader(self, ctx: commands.Context, *, pride_leader_name: Optional[str]) -> None: """Provides info about pride leader by taking name as input or randomly without input.""" if pride_leader_name is None: name_list = [name for name in self.pride] leader = random.choice(name_list) else: leader = self.name_verifier(pride_leader_name) if leader is None: invalid_embed = self.invalid_embed_generate(pride_leader_name) await ctx.send(embed=invalid_embed) return pride_leader_embed = self.embed_builder(leader) await ctx.send(embed=pride_leader_embed)

You can do something similar(do test it).

quanta-kt · 2020-10-15T10:33:26Z

+                valid_name.append(name)
+
+        if not valid_name:
+            valid_name = ", ".join(name for name in self.pride.keys())


You can simply call join on the dict keys.

Suggested change

valid_name = ", ".join(name for name in self.pride.keys())

valid_name = ", ".join(self.pride.keys())

quanta-kt · 2020-10-15T10:33:32Z

+            valid_name = ", ".join(name for name in self.pride.keys())
+            error_msg = "Sorry your input didn't match any stored name, here is a list of available names"
+        else:
+            valid_name = "\n".join(name for name in valid_name)


Likewise here-

Suggested change

valid_name = "\n".join(name for name in valid_name)

valid_name = "\n".join(valid_name)

Shivansh-007 · 2021-01-19T05:49:44Z

+            log.trace("Got Valid name.")
+            return leader_name
+
+    def invalid_embed_generate(self, pride_leader: str) -> discord.Embed:


Hey @Anubhav1603 ,
Would it be good, if we write a logging statement here, so that we can know which pride leaders are users interested in, and according to the requests, we could keep adding information to our prideleader.json file, making it more intelligent.

Shivansh-007 · 2021-01-19T05:52:41Z

+        valid_name = []
+        pride_leader = pride_leader.title()
+        for name in self.pride:
+            if fuzz.ratio(pride_leader, name) >= 40:


Adding 40 as a class variable would be better, so it's easily modified from one place.

Shivansh-007 · 2021-01-19T05:55:04Z

+        embed.title = f'__{leader_name}__'
+        embed.description = self.pride[leader_name]["About"]
+        embed.add_field(name="__Known for__", value=self.pride[leader_name]["Known for"], inline=False)
+        embed.add_field(name="__D.O.B and Birth place__", value=self.pride[leader_name]["Born"], inline=False)
+        embed.add_field(name="__Awards and honors__", value=self.pride[leader_name]["Awards"], inline=False)


Suggested change

embed.title = f'__{leader_name}__'

embed.description = self.pride[leader_name]["About"]

embed.add_field(name="__Known for__", value=self.pride[leader_name]["Known for"], inline=False)

embed.add_field(name="__D.O.B and Birth place__", value=self.pride[leader_name]["Born"], inline=False)

embed.add_field(name="__Awards and honors__", value=self.pride[leader_name]["Awards"], inline=False)

embed.title = leader_name

embed.description = self.pride[leader_name]["About"]

embed.add_field(name="Known for", value=self.pride[leader_name]["Known for"], inline=False)

embed.add_field(name="D.O.B and Birth place", value=self.pride[leader_name]["Born"], inline=False)

embed.add_field(name="Awards and honors", value=self.pride[leader_name]["Awards"], inline=False)

Underlines aren't needed, as the field titles and embed titles have different font sizes making them easily differentiable. Just like tagptroll1 mentioned

Xithrius · 2021-01-24T14:57:24Z

@Anubhav1603 will you be continuing this PR?

Xithrius · 2021-03-06T13:44:10Z

If anyone would like to pick up this PR, feel free to do so. Author doesn't look to be active anymore.

Shivansh-007 · 2021-03-06T16:16:16Z

I have taken over this PR and will be opening a new one, mentioning changes requested here.

We can close this one.

Anubhav1603 added 7 commits October 5, 2020 16:05

added pride leader command

4e52be8

added pride leader json

f37be5a

added more info

f0f7b35

removed str from dict

2842798

added .

235796d

added more info in json

51900f8

corrected docstring and added docstring

5406476

Anubhav1603 requested a review from a team as a code owner October 6, 2020 12:35

Anubhav1603 requested review from scragly and tagptroll1 October 6, 2020 12:35

ghost added the needs 2 approvals label Oct 6, 2020

Akarys42 added hacktoberfest-accepted type: feature Relating to the functionality of the application. labels Oct 6, 2020

tagptroll1 suggested changes Oct 6, 2020

View reviewed changes

ghost added status: waiting for author Waiting for author to address a review or respond to a comment and removed needs 2 approvals labels Oct 6, 2020

Corrected doc string

5bf8221

Co-authored-by: Thomas Petersson <Thomas@petersson.priv.no>

ghost added needs 2 approvals and removed status: waiting for author Waiting for author to address a review or respond to a comment labels Oct 6, 2020

Anubhav1603 and others added 5 commits October 6, 2020 20:11

Cleaned up the cog

da24034

Co-authored-by: Thomas Petersson <Thomas@petersson.priv.no>

Cleaned up the cog

eae74b6

Co-authored-by: Thomas Petersson <Thomas@petersson.priv.no>

Corrected error msg

9359097

Co-authored-by: Thomas Petersson <Thomas@petersson.priv.no>

removed keys()

575bf39

Co-authored-by: Thomas Petersson <Thomas@petersson.priv.no>

added title ()

74662ae

Co-authored-by: Thomas Petersson <Thomas@petersson.priv.no>

RohanJnr suggested changes Oct 7, 2020

View reviewed changes

Comment thread bot/exts/pride/prideleader.py Outdated

ghost added status: waiting for author Waiting for author to address a review or respond to a comment and removed needs 2 approvals labels Oct 7, 2020

Update bot/exts/pride/prideleader.py

dd7b24c

Co-authored-by: Rohan Reddy Alleti <roalleti@gmail.com>

ghost removed the status: waiting for author Waiting for author to address a review or respond to a comment label Oct 7, 2020

Anubhav1603 added 2 commits October 11, 2020 18:13

added t.optional

3b86c27

added default value to None in command

a325ea0

RohanJnr suggested changes Oct 15, 2020

View reviewed changes

Comment thread bot/exts/pride/prideleader.py Outdated

ghost added status: waiting for author Waiting for author to address a review or respond to a comment and removed needs 2 approvals labels Oct 15, 2020

Corrected annotation

19739e9

Co-authored-by: Rohan Reddy Alleti <rohanjnr44@gmail.com>

ghost added needs 2 approvals and removed status: waiting for author Waiting for author to address a review or respond to a comment labels Oct 15, 2020

RohanJnr reviewed Oct 15, 2020

View reviewed changes

quanta-kt reviewed Oct 15, 2020

View reviewed changes

Merge branch 'master' into prideleader

b91ab71

github-actions Bot added the needs core dev approval label Nov 30, 2020

Shivansh-007 suggested changes Jan 19, 2021

View reviewed changes

Akarys42 removed needs core dev approval labels Feb 20, 2021

Xithrius added priority: 2 - normal area: backend Related to internal functionality and utilities status: waiting for author Waiting for author to address a review or respond to a comment status: stale Has had no activity for a while and removed priority: 2 - normal labels Feb 23, 2021

Xithrius added up for grabs Available for anyone to work on and removed hacktoberfest-accepted status: stale Has had no activity for a while status: waiting for author Waiting for author to address a review or respond to a comment labels Mar 6, 2021

Shivansh-007 mentioned this pull request Mar 6, 2021

Tech Leaders Pride #208

Closed

Shivansh-007 mentioned this pull request Mar 8, 2021

Tech Leaders Pride #614

Merged

2 tasks

Xithrius closed this Mar 10, 2021



		class PrideLeader(commands.Cog):
		"""Gives a Pride Leader Info."""

	"""Gives a Pride Leader Info."""
	"""Gives information about Pride Leaders."""



		def setup(bot: commands.Bot) -> None:
		"""Cog loader for drag queen name generator."""

	valid_name = ", ".join(name for name in self.pride.keys())
	valid_name = ", ".join(self.pride.keys())

	valid_name = "\n".join(name for name in valid_name)
	valid_name = "\n".join(valid_name)

Uh oh!

Conversation

Anubhav1603 commented Oct 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Relevant Issues

Description

Screenshots

Did you:

Uh oh!

tagptroll1 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Xithrius commented Jan 24, 2021

Uh oh!

Xithrius commented Mar 6, 2021

Uh oh!

Shivansh-007 commented Mar 6, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Anubhav1603 commented Oct 6, 2020 •

edited

Loading