Skip to content
This repository was archived by the owner on Mar 14, 2021. It is now read-only.

Team 11 #11

Open
wants to merge 68 commits into
base: master
Choose a base branch
from
Open

Team 11 #11

wants to merge 68 commits into from

Conversation

delterr
Copy link

@delterr delterr commented Mar 23, 2018

No description provided.

Pipfile Outdated
@@ -8,6 +8,9 @@ name = "pypi"
aiodns = "*"
aiohttp = "<2.3.0,>=2.0.0"
websockets = ">=4.0,<5.0"
"beautifulsoup4" = "*"
requests-html = "*"
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like you've edited this file by hand. This is bad practise, but at least you've updated the lockfile as well.

Please be careful about using requests - it's not asynchronous.

Copy link
Author

Choose a reason for hiding this comment

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

I am 100% sure that I did not update the Pipfile by hand. I used pipenv install requests-html

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay. Either way, we've made a statement about the wikipedia module on Discord, and the same applies to Requests. Requests does not support asyncio, and you're going to want to avoid any solution that doesn't.

delterr and others added 27 commits March 23, 2018 15:48
[UPDATE] Ignored a silly rule from flake8.
# Please enter a commit message to explain why this merge is necessary,
# especially if it merges an updated upstream into a topic branch.
#
# Lines starting with '#' will be ignored, and an empty message aborts
# the commit.

Adding random snakes
Adding W503 and E226 to flake8 ignore list.
.gitignore Outdated
@@ -9,6 +9,8 @@ __pycache__/
# Distribution / packaging
.Python
env/
.idea/
.env
Copy link
Contributor

Choose a reason for hiding this comment

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

Aren't these both already in the gitignore?

Pipfile Outdated
@@ -8,6 +8,8 @@ name = "pypi"
aiodns = "*"
aiohttp = "<2.3.0,>=2.0.0"
websockets = ">=4.0,<5.0"
"beautifulsoup4" = "*"
wikipedia = "*"
Copy link
Contributor

Choose a reason for hiding this comment

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

If you're not using this, you should pipenv uninstall it.

@@ -1,8 +1,10 @@
# coding=utf-8
import logging
import logging, aiohttp, random, wikipedia
Copy link
Contributor

Choose a reason for hiding this comment

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

No, you should be using separate lines, and they should be in alphabetical order.

@@ -11,8 +13,22 @@ class Snakes:
"""
Snake-related commands
"""
python_info = '''
Python (Programming Language)
Copy link
Contributor

Choose a reason for hiding this comment

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

You should probably remove all these huge indents.

Copy link
Contributor

Choose a reason for hiding this comment

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

You also don't need the \ns in here. Those are taken from how you've formatted the string.

@@ -28,9 +44,23 @@ def __init__(self, bot: AutoShardedBot):
:param name: Optional, the name of the snake to get information for - omit for a random snake
:return: A dict containing information on a snake
"""
name = str(name)
Copy link
Contributor

Choose a reason for hiding this comment

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

No, don't do this. If there's no snake name passed, this will now be "None", which is annoying to check for.

if name.lower() == 'python':
name = self.python_info

return name
Copy link
Contributor

Choose a reason for hiding this comment

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

This method doesn't work at all. Return name? Are you still working on this?

@@ -40,8 +70,93 @@ def __init__(self, bot: AutoShardedBot):
:param ctx: Context object passed from discord.py
:param name: Optional, the name of the snake to get information for - omit for a random snake
"""
# await ctx.send(BeautifulSoup(text, 'lxml').find("title"))
name = str(name)
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, why are you doing this?

title = soup.find('h1').text
description = soup.find('table').text

em = discord.Embed(title=title, description=description)
Copy link
Contributor

Choose a reason for hiding this comment

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

While you should be creating the embed in this method, you should be retrieving the information over in get_snek().


# Any additional commands can be placed here. Be creative, but keep it to a reasonable amount!
@command()
async def snake(self, ctx: Context, x=50, y=30):
Copy link
Contributor

Choose a reason for hiding this comment

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

Really fun idea. But maybe a little ambitious for you guys.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is super hard to read, I'm going to have to test it to really see what it does. Maybe you could comment it?

Also, perhaps you could use reactions via wait_for_reactions() instead of a list of inputs?

@gdude2002
Copy link
Contributor

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants