Team 21 by eivl · Pull Request #12 · python-discord/code-jam-1

eivl · 2018-03-23T10:30:11Z

… more testing but not code.

gdude2002

I see no changes here - did you forget to push?

commit ba5705b8e9e18d9f8a541dee446da21002853d37 Author: TildeBeta <tildebetabot@gmail.com> Date: Sun Mar 25 14:57:00 2018 +0300 Flake8 fixes and special case handling commit 051952e Author: TildeBeta <tildebetabot@gmail.com> Date: Sun Mar 25 01:01:39 2018 +0200 Fix disambiguator, create a mock embed commit d0d944f Author: TildeBeta <tildebetabot@gmail.com> Date: Sun Mar 25 00:25:25 2018 +0200 Fix up malformed data commit d8d9894 Author: TildeBeta <tildebetabot@gmail.com> Date: Sat Mar 24 20:09:42 2018 +0200 Redundant set cast commit 0984643 Author: TildeBeta <tildebetabot@gmail.com> Date: Sat Mar 24 19:33:29 2018 +0200 Improve disambiguation commit 2124795 Author: TildeBeta <tildebetabot@gmail.com> Date: Sat Mar 24 15:26:55 2018 +0200 Paginate results instead of posting a massive block commit c6112d5 Author: TildeBeta <tildebetabot@gmail.com> Date: Sat Mar 24 14:29:41 2018 +0200 Improve snake name matching

[UPDATE] Ignored a silly rule from flake8.

eivl · 2018-03-25T17:49:28Z

so does it work now?

eivl · 2018-03-25T19:18:58Z

flake8 error fixed

eivl

file not needed.

gdude2002 · 2018-03-25T21:33:25Z

bot/cogs/snakes.py

        """
+        snake_info = {}
+        # python (programming language) pageid = 23862
+        URL = "https://en.wikipedia.org/w/api.php?"


Constants should probably be at module-level?

gdude2002 · 2018-03-25T21:34:57Z

bot/cogs/snakes.py

+        PAGE_ID_URL = f"{URL}{FORMAT}&{ACTION}&{LIST}&{SRSEARCH}{name}&{UTF8}&{SRLIMIT}"
+
+        async with aiohttp.ClientSession() as session:
+            j = await self.fetch(session, PAGE_ID_URL)


Not a fan of one-letter variable names.

gdude2002 · 2018-03-25T21:36:11Z

bot/cogs/snakes.py

+                          '.svg',
+                          'ange.',
+                          'Adder%20(PSF).png'
+                          ]


These are hilariously arbitrary. What's up? :P

Wikipedia has arbitrary images on their snake pages ¯\_(ツ)_/¯

gdude2002 · 2018-03-25T21:37:30Z

bot/cogs/snakes.py

+                          'Adder%20(PSF).png'
+                          ]
+                for image in snake_info["images"]:
+                    i = image["title"].split(':')[1].replace(" ", "%20")


Again, I don't love these one-letter variable names

gdude2002 · 2018-03-25T21:38:10Z

bot/cogs/snakes.py

+            title=data['title'],
+            description=match.group(1) if match else None,
+            url=data['fullurl'],
+            colour=0x59982F


What colour is this? Is it not included in the discord.Colour class as a staticmethod?

No it's just a random green I found online

gdude2002 · 2018-03-25T21:41:48Z

bot/decorators.py

    return commands.check(predicate)
+
+
+def locked():


Can you comment this? It's not obvious what it does

lemonsaurus

some minor change requests

lemonsaurus · 2018-03-25T21:36:41Z

bot/cogs/snakes.py

+        PAGE_ID_URL = f"{URL}{FORMAT}&{ACTION}&{LIST}&{SRSEARCH}{name}&{UTF8}&{SRLIMIT}"
+
+        async with aiohttp.ClientSession() as session:
+            j = await self.fetch(session, PAGE_ID_URL)


j is not acceptable as a variable name for this.

lemonsaurus · 2018-03-25T21:37:18Z

bot/cogs/snakes.py

+            try:
+                PAGEID = j["query"]["search"][0]["pageid"]
+            except KeyError:
+                PAGEID = 41118


this could use a comment, what the hell is it

lemonsaurus · 2018-03-25T21:43:34Z

bot/cogs/snakes.py

+                          'Adder%20(PSF).png'
+                          ]
+                for image in snake_info["images"]:
+                    i = image["title"].split(':')[1].replace(" ", "%20")


this line is far too dense, and i is another awful variable name.

lemonsaurus · 2018-03-25T21:51:01Z

bot/cogs/snakes.py

+                    if not i.startswith('Map'):
+                        for b in banned:
+                            image_banned = False
+                            if b in i:


and this line pretty much perfectly illustrates why single letter variables suck.

lemonsaurus · 2018-03-25T21:52:50Z

bot/decorators.py

    return commands.check(predicate)
+
+
+def locked():


please docstring the decorators as well. I shouldn't have to study this to figure out what that decorator does. (I don't think it's immediately obvious from the name, either)

lemonsaurus · 2018-03-25T22:13:02Z

bot/cogs/snakes.py

+                          '.svg',
+                          'ange.',
+                          'Adder%20(PSF).png'
+                          ]


please be consistent about this style choice. Later on you're using the convention that looks like this:

var = ( "contents" )

This is the style used in all of our open source projects, so we would prefer if you used this style here as well.

Zwork101

Amazing work! You seem to like to have hard coded values and urls in your function. I would move that right next to you regex compiles. Other then that, great bot! See you next code jam!

Zwork101 · 2018-04-03T22:00:05Z

bot/cogs/snakes.py

    def __init__(self, bot: AutoShardedBot):
        self.bot = bot

-    async def get_snek(self, name: str = None) -> Dict[str, Any]:


Why remove what it returns? That's great documenting!

Zwork101 · 2018-04-03T22:03:41Z

bot/cogs/snakes.py

-        """
-        Go online and fetch information about a snake
+        async with aiohttp.ClientSession() as session:
+            params = {


These don't have to be defined each time it's called, maybe class property?

Zwork101 · 2018-04-03T22:04:00Z

bot/cogs/snakes.py

+
+            # constructing dict - handle exceptions later
+            try:
+                snake_info["title"] = json["query"]["pages"][f"{pageid}"]["title"]


Use loops! That would clean up this hunk of code

Zwork101 · 2018-04-03T22:04:53Z

bot/cogs/snakes.py

+                thumb_list = []
+
+                # Wikipedia has arbitrary images that are not snakes
+                banned = [


Once again, hard coded lists should be moved to somewhere such as a class variable

Zwork101 · 2018-04-03T22:05:24Z

bot/cogs/snakes.py

+                for image in snake_info["images"]:
+                    # images come in the format of `File:filename.extension`
+                    file, sep, filename = image["title"].partition(':')
+                    filename = filename.replace(" ", "%20")  # Wikipedia returns good data!


Use the built-in urllib for this!

Zwork101 · 2018-04-03T22:07:51Z

bot/cogs/snakes.py

+
+        embed.set_footer(text='Powered by Wikipedia')
+
+        emoji = 'https://emojipedia-us.s3.amazonaws.com/thumbs/60/google/3/snake_1f40d.png'


Yak! Hard coded value inside the function again. Same issue as the list and dicts.

testfile for testing commiting inside my text editor. can be used for…

cd2958a

… more testing but not code.

gdude2002 changed the title ~~testfile for testing commiting inside my text editor. can be used for…~~ Team 22 Mar 23, 2018

Naive input handling

ed78916

gdude2002 suggested changes Mar 25, 2018

View reviewed changes

eivl and others added 25 commits March 25, 2018 13:56

get_snake info from wikipedia, get_snek missing exception handeling

0a8ecf2

Merge pull request #1 from discord-python/master

b41ebe6

[UPDATE] Ignored a silly rule from flake8.

Quick fixes

e10dfd9

Merge branch 'master' of github.com:eivl/code-jam-1

5f24356

Fix flake8 import order issue

33df4d7

Make import PEP8 after previous change

38dadc2

exception handeling, pageid->wiki_error & dict -> add error key

9dd9c2f

Merge branch 'master' of https://github.com/eivl/code-jam-1

8e49271

changed to bot.http_session

6be2ba0

map_list and image_list -> full url support

a88e00e

Various fixes

83a1019

edited code to pure json return instead of text

128efbf

Turns out this broke the custom invoking

01529d4

Oops

280bfa9

replaced spacees with %20 in snake image URL

316b55c

added thumbnail list to snake dict with 100px width

20c555a

Embed thumbnail

f9cf749

Quick fixes

525adc0

New secret command

f844893

Remove test file

a143ae3

Better safe than sorry

5e7ed88

Reuse ClientSession

676f1ec

Add clarification

f2e5d69

New command and disambiguate changes

92bc40a

TildeBeta and others added 5 commits March 25, 2018 21:28

Docstring for new command

9373d79

banned images from guess

69ab790

Merge branch 'master' of https://github.com/eivl/code-jam-1

e579d2a

more banned images

bfdc89c

flake8 error in import statement, commented out

98b8ba7

eivl commented Mar 25, 2018

View reviewed changes

eivl and others added 7 commits March 25, 2018 21:35

forcing travis rebuild

f741a2b

newline error flake8 fixed

a52208a

commands changed to bot.snakes.COMMAND (from bot.COMMAND)

3e7f499

README info

cc8624a

README update

11be7f1

README fix

53973e4

Allow user to only run one instance of command at a time

ce51e6c

gdude2002 changed the title ~~Team 22~~ Team 21 Mar 25, 2018

Fix import order

812e4c2

gdude2002 suggested changes Mar 25, 2018

View reviewed changes

Document locked decorator

cf32fdb

lemonsaurus suggested changes Mar 25, 2018

View reviewed changes

Clean up get_snek a bit

7e5a33c

lemonsaurus approved these changes Mar 25, 2018

View reviewed changes

Zwork101 reviewed Apr 3, 2018

View reviewed changes


		embed.set_footer(text='Powered by Wikipedia')

		emoji = 'https://emojipedia-us.s3.amazonaws.com/thumbs/60/google/3/snake_1f40d.png'

Uh oh!

Conversation

eivl commented Mar 23, 2018

Uh oh!

gdude2002 left a comment

Choose a reason for hiding this comment

Uh oh!

eivl commented Mar 25, 2018

Uh oh!

eivl commented Mar 25, 2018

Uh oh!

eivl left a comment

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!

TildeBeta Mar 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

lemonsaurus left a comment

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!

Zwork101 left a comment

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

TildeBeta Mar 25, 2018 •

edited

Loading