[APPROVED]Dronefly

Applications
#1

Discord Name:

SyntheticBee#4951

GitHub Repository (Must be V3):

Description:

A cog collection for naturalists.

The principal cog, inatcog, accesses the iNaturalist platform API to explore the data, with a particular focus of social features important to conversations about the taxa and species being observed by the iNaturalist Discord community. It is in an alpha phase of rapid change & building out of new features with no “stable” branch established yet.

A second cog, ebirdcog, accesses the eBird platform API. It is less well developed, but there is some interest from the community to help make it better. It is therefore fairly stable, but should also be considered “alpha” for the time being.

#2
#3

Hi SyntheticBee, thank you for your patience while I review your repo. While you only have 2 cogs your inatcog is more than complex enough to make up for our 3 cog minimum. I did notice a few things while going through your repo though and have listed them below. Once they’re cleaned up I’ll be happy to mark this as approved.

Commit hash at time of review - 7a9242fa8965122f5fc210fe57f8292573925dee

ebirdcog

api.py

  • Line 21: You don’t check if “api_key” exists from the key causing a KeyError.

inatcog

listeners.py

  • Line 29-43: Your “FIXME” comment can be solved by replacing on_message with on_message_without_command alternatively you can get the bots prefix list by doing await self.bot.get_valid_prefixes() or await self.bot.get_prefix(message.channel). In testing I wasn’t quite sure this was even working as intended as I coudln’t seem to get a response when I enabled it in a channel only. Unless I’m misunderstanding the true intent of this function.

taxa.py

  • Lines 19, 21, 22, 23, 27, 240-242: These should be using typing.Optional for the type hints. For example common: Optional[str] This is the same as doing common: Union[str, None] which appears to be what you’re trying to do here by suggesting the type is a string or NoneType.
#4

Thanks for taking the time to handle my review.

  • ebirdcog/api.py Fixed.
  • inatcog/listeners.py I’m not sure any of those suggested solutions would do what I want. The intent is to ignore not just commands that are intended for this bot instance, but commands intended for any bot instance on the same server (e.g. production bot Dronefly & development bot CuckooBee are often present together). Thoughts on how I’d fix that in a general way? Or have I overlooked something & one of those approaches would actually do that?
  • inatcog/taxa.py Fixed.
#5

I would still recommend utilizing on_message_without_command at least for the current instance to avoid the check locally. As for checking another instance your best bet would be permission based in the server for the bots to see specific channels or some other more customizable filter which you can then add and remove via command. You can then load the set list of bot prefixes from other bots into the regex pattern stored in memory by joining the prefixes like so

prefixes = r"|".join(f"\{p}" for p in await self.config.bot_prefixes())
prefix_pattern = re.compile(r"^{prefixes}".format(prefixes=prefixes))
# I reversed the logic here because this makes more sense to me
# the way you have it would also work except for the unescaped `/`
if autoobs and not prefix_pattern.match(message.content):

This way it can be dynamically adjusted for more than just one extra bot on the server you want to avoid and for anyone else who might want to use it to prevent the action when certain bot commands are used.

Anyways I’m marking this as approved, these are some quite impressive cogs!

1 Like
#6
#7