Team 20 by isik-kaplan · Pull Request #16 · python-discord/code-jam-1

isik-kaplan · 2018-03-24T03:01:06Z

Write some gibberish -Bisk 24.03.2018

Fix Pipfile dependencies

schwartzadev

this looks good!
I would check on the Travis CI report and fix the changes that the linter found

schwartzadev · 2018-03-24T03:19:48Z

bot/cogs/snakes.py

        self.bot = bot

+    @staticmethod
+    def snake_url(name) -> str:


type hint name here

schwartzadev · 2018-03-24T03:22:18Z

bot/cogs/snakes.py

+            thumb = content.get("thumbnail", {}).get("source", "http://i.imgur.com/HtIPyLy.png/beep")
+            image = "/".join(thumb.replace("thumb/", "").split("/")[:-1])
+
+            # WHY WOULD YOU USE DICTIONARY LITERAL IN A RETURN STATEMENT but okay lol


this seems fine from the way you've done this - not sure I have any qualms with it but maybe consider making a class for this return object...

schwartzadev · 2018-03-24T03:24:37Z

bot/cogs/snakes.py

+        em = Embed()
+        em.title = content["name"]
+        em.description = content["info"][:1970] + "\n\nPS. If the image is a fucking map, blame wikipedia. -Somejuan"
+        em.set_image(url=content["image"])


some of this can be more concise:

embed=discord.Embed( title=content["name"], description=content["info"][:1970] + "\n\nPS. If the image is a fucking map, blame wikipedia. -Somejuan" )

schwartzadev · 2018-03-24T03:25:12Z

run.py

-bot.run(os.environ.get("BOT_TOKEN"))
+#bot.run(os.environ.get("BOT_TOKEN"))
+# I SWEAR TO GOD, HONESTLY WATCH ME LEAVE THIS TOKEN IN LMAO
+bot.run('BOT TOKEN')


How does this run properly?

lemonsaurus

A couple of things here to look at, but it could be worse. Looking forward to the finished product.

lemonsaurus · 2018-03-24T03:15:26Z

Pipfile

 "72eb2aa" = {file = "https://github.com/Rapptz/discord.py/archive/rewrite.zip"}
 aiodns = "*"
-aiohttp = "<2.3.0,>=2.0.0"
+aiohttp = "*"


any particular reason this was done? discord.py doesn't like the latest aiohttp, we locked the version for a reason.

lemonsaurus · 2018-03-24T03:23:55Z

bot/cogs/snakes.py

+    "image": "https://www.python.org/static/community_logos/python-logo-master-v3-TM-flattened.png"
+}
+with open("bot/snakes.txt") as file:
+    SNAKES = list(map(lambda ln: ln.strip('\n'), file.readlines()))


I don't even know where to begin with this line.

don't use map and lambdas, use list comps. this isn't Python 1.0.

a simple ln.strip() will also remove newlines (and spaces) from the start and end. why even specify the \n?

don't call the file variable file, that's a python 2 builtin which means syntax highlighting looks funky in most editors.

ln is a shitty variable name. just call it line. this isn't code golf.

lemonsaurus · 2018-03-24T03:24:51Z

bot/cogs/snakes.py

 log = logging.getLogger(__name__)

+# Probably should move these somewhere
+BASEURL = "https://en.wikipedia.org/w/api.php?format=json&action=query&prop=extracts|pageimages&exintro=&explaintext=&titles={}&redirects=1"


this line is 140 characters. it's not gonna pass linting and it's just not okay to do even if it did.

lemonsaurus · 2018-03-24T03:25:50Z

bot/cogs/snakes.py

+from discord import Embed
+
+import aiohttp
+import json


these imports aren't gonna pass linting. wrong import orders and groupings. you should fix them so they are PEP8 compliant. Why aren't you guys using flake8?

lemonsaurus · 2018-03-24T03:27:14Z

bot/cogs/snakes.py

        self.bot = bot

+    @staticmethod
+    def snake_url(name) -> str:


name should be typehinted.

lemonsaurus · 2018-03-24T03:37:39Z

bot/cogs/snakes.py

+            thumb = content.get("thumbnail", {}).get("source", "http://i.imgur.com/HtIPyLy.png/beep")
+            image = "/".join(thumb.replace("thumb/", "").split("/")[:-1])
+
+            # WHY WOULD YOU USE DICTIONARY LITERAL IN A RETURN STATEMENT but okay lol


returning a dictionary literal seems fine. this comment, however, does not.

lemonsaurus · 2018-03-24T03:38:23Z

bot/cogs/snakes.py

+            # WHY WOULD YOU USE DICTIONARY LITERAL IN A RETURN STATEMENT but okay lol
+            return {
+                "name": content["title"],
+                "info": content.get("extract", "") or "I don't know about that snake!",


don't do this. the second param of .get is what to return if the get fails.

"info": content.get("extract", "I don't know about that snake!"),

lemonsaurus · 2018-03-24T03:40:21Z

bot/cogs/snakes.py

+        # Just a temporary thing to make sure it's working
+        em = Embed()
+        em.title = content["name"]
+        em.description = content["info"][:1970] + "\n\nPS. If the image is a fucking map, blame wikipedia. -Somejuan"


other teams seem to have found a way not to return maps. :P

Indeed they have, but we aren't other teams :D

lemonsaurus · 2018-03-24T03:41:03Z

bot/cogs/snakes.py

-        should be sent back to Discord in an embed.
+        content = await self.get_snek(name)
+        # Just a temporary thing to make sure it's working
+        em = Embed()


there's no two letter maxlimit on variable names. what's wrong with embed = Embed()?

lemonsaurus · 2018-03-24T03:41:39Z

bot/cogs/snakes.py

-        Go online and fetch information about a snake
+        """Get a snake with a given name, or otherwise randomly"""
+        if name is None:
+            name = random.choice([x.strip('\n') for x in SNAKES])


didn't you already strip out the newlines when you created SNAKES?

[UPDATE] Ignored a silly rule from flake8.

This reverts commit a626e56.

This reverts commit aa43444.

This reverts commit a626e56.

gdude2002 · 2018-03-25T10:10:26Z

Pipfile

 "72eb2aa" = {file = "https://github.com/Rapptz/discord.py/archive/rewrite.zip"}
 aiodns = "*"
-aiohttp = "<2.3.0,>=2.0.0"
+aiohttp = "==2.0.0"


gdude2002 · 2018-03-25T10:12:02Z

bot/cogs/snakes.py

@@ -1,11 +1,39 @@
 # coding=utf-8
+
+import async_timeout


This isn't declared in your Pipfile.

gdude2002 · 2018-03-25T10:15:32Z

bot/cogs/snakes.py

-        Go online and fetch information about a snake
+    @staticmethod
+    def parse_kwarg(kwarg: str):
+        """Return the value of a kwarg (needs manual assignment)"""


I don't understand - what is this supposed to be doing, and why?

gdude2002 · 2018-03-25T10:16:03Z

bot/cogs/snakes.py

+            return BASEURL.format(urllib.parse.quote_plus(text))
+
+        # Check if the snake name is valid
+        if name.upper() in [name.upper() for name in SNAKES]:


Instead of upper()ing all the names every time you do this, maybe they could be stored that way?

gdude2002 · 2018-03-25T10:19:21Z

bot/cogs/snakes.py

+    @staticmethod
+    async def fetch(session, url: str):
+        """Fetch the contents of a URL as a json"""
+        async with async_timeout.timeout(10):


Why not just with aiohttp.Timeout(10):?

gdude2002 · 2018-03-25T10:25:44Z

bot/cogs/snakes.py

-        """
-        Go online and fetch information about a snake
+    async def get(self, ctx: Context, name: str = None, autocorrect: str = None, details: str = None):
+


You're missing a docstring here.

gdude2002 · 2018-03-25T10:26:16Z

run.py

 bot.load_extension("bot.cogs.snakes")

 bot.run(os.environ.get("BOT_TOKEN"))
+#bot.run('bot token goes here for testing')


You should remove this line.

We explained exactly how this works in the docs.

gdude2002 · 2018-03-25T10:27:06Z

snakes.txt

+wikipedia
+help
+wikipedia
+help


Should these be in there?

gdude2002 · 2018-03-25T10:27:20Z

snakes.txt

+wutu
+yarara
+list of snakes
+list of reptiles


I imagine these listing pages don't need to be here either

gdude2002 · 2018-03-25T10:27:48Z

snakes2.py

@@ -0,0 +1,150 @@
+# coding=utf-8


Why is there another snakes solution file here? I'm not going to review both - pick one and work on it together

gdude2002 · 2018-03-25T13:02:28Z

run.py


-bot.run(os.environ.get("BOT_TOKEN"))
+#bot.run(os.environ.get("BOT_TOKEN"))
+bot.run('NDE0MDk5NTIyOTMxNzg1NzI4.DZiQjA.3eA2gcYy6D8KI8IhSfCr87oDePc')


You've committed your bot token to the repository. Go to you bot's application page, invalidate the token, and then read over the bot setup guide again - add your token to the .env file like we described, and return this line to how it was when we gave it to you.

gdude2002 · 2018-03-25T16:36:58Z

bot/cogs/snakes.py

@@ -1,49 +0,0 @@
-# coding=utf-8


Ya want to explain this? Why delete that file?

This reverts commit 8bb749d.

This reverts commit 80e01f6.

gdude2002 · 2018-03-25T21:52:05Z

You code is failing to lint. Please see Travis for more information.

skrungly and others added 10 commits March 23, 2018 01:20

Merge pull request #1 from discord-python/master

ece1140

Fix Pipfile dependencies

First draft

54fbeb0

Adding snakes.txt

105b415

Cleaned up a bit, added bot test

fd9db0d

Added pictures

63ccb69

Embeds and redirects :D

3763038

Fixed url encoder

47e03d6

I don't fucking know

9b4a683

I don't fucking know

73ab47b

Fixed up wikipedia's bullshit

2cbbf34

isik-kaplan changed the title ~~I don't know what to write~~ Team 20 Mar 24, 2018

schwartzadev reviewed Mar 24, 2018

View reviewed changes

lemonsaurus reviewed Mar 24, 2018

View reviewed changes

isik-kaplan and others added 15 commits March 24, 2018 08:05

fixed little stuff

0ddc2a4

Various small edits

e766154

Various small edits

8d89ccf

fixed more little stuff

002dcfa

Merge pull request #2 from discord-python/master

163d4d7

[UPDATE] Ignored a silly rule from flake8.

Added autocomplete kwarg(ish)

916a7ac

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

9338eeb

kwarg parsing ^^

a626e56

fucking hell, had accidentally pushed bot token :I

aa43444

Revert "kwarg parsing ^^"

532995e

This reverts commit a626e56.

Revert "fucking hell, had accidentally pushed bot token :I"

992de22

This reverts commit aa43444.

Revert "kwarg parsing ^^"

305c3fe

This reverts commit a626e56.

Well I'm stupid :D

d419d1c

snakes.txt + snakes2.py

108e84d

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

971d098

gdude2002 suggested changes Mar 25, 2018

View reviewed changes

Added snake_card command (kinda)

2c6af66

gdude2002 reviewed Mar 25, 2018

View reviewed changes

kingdom5500 added 3 commits March 25, 2018 14:40

These files are useful

80e01f6

Finally used .env

8bb749d

Arg edits, made by someone

fc5dddb

gdude2002 suggested changes Mar 25, 2018

View reviewed changes

bot/cogs/snakes.py

@@ -1,49 +0,0 @@

# coding=utf-8

Copy link

Contributor

gdude2002 Mar 25, 2018

Choose a reason for hiding this comment

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

Ya want to explain this? Why delete that file?

isik-kaplan and others added 7 commits March 25, 2018 19:48

added snkaes.py *duuh*

3330e52

Oops.

1b5e682

Oops.

2344edc

Revert "Finally used .env"

94bfad1

This reverts commit 8bb749d.

Revert "These files are useful"

a93cc3b

This reverts commit 80e01f6.

Touched up some stuff

9f77c25

Accidentally pushed old token fucking hell

626b8b6

kingdom5500 added 5 commits March 26, 2018 00:19

Finishing touches for now. Will still need fine-tuning though

d31e269

Added my own version. Someone, don't hurt me :(

78ad193

Added a comment for clarification :D

e395e6b

Improved special cases

93d1d8e

Improved special cases

b9ee9c0

Uh oh!

Conversation

isik-kaplan commented Mar 24, 2018

Uh oh!

schwartzadev left a comment • 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!

schwartzadev Mar 24, 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!

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!

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gdude2002 commented Mar 25, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

schwartzadev left a comment •

edited

Loading

schwartzadev Mar 24, 2018 •

edited

Loading