you're using ConEmu right, numerlor?
#dev-contrib
molten bough
if you have a running process that takes input - like, eg, an ssh session - does conemu let you close it?
green oriole
_Lemme boot up my windows _
brazen charm
I think it's detecting win and assuming pycharm's terminal is a normal one and trying to do its fancy stuff
With a warning I think
molten bough
I wonder if they found a workaround then
every terminal I've used that isn't cmd will hang if you do that
brazen charm
https://i.imgur.com/ymO7eeP.png after an input() in the repl
molten bough
I mean, click yes ofc
brazen charm
just closes it normally
green oriole
Works fine for me
Owww, I need to update my whole config file now
Time to go back to linux
crude gyro
I've written several
@molten bough
yeah, I meant one in one of our repos. if it doesn't work for our windows users and it's not fixable we may need to e.g. put in an override env var or something.
molten bough
Oh, right
I'll look at the actual library used first, there might be something
okay yeah, looks like COLOREDLOGS_FIELD_STYLES and COLOREDLOGS_LEVEL_STYLES can both be set to the empty string
That'd do it
so it might be worth just documenting that
crude gyro
sounds reasonable, at least if anyone else has an issue with this.
brazen charm
does it work in pycharm on non win?
after looking into it further it looks like it doesn't do colors without emulating the terminal because the output is redirected and it doesn't do colors when sys.stdout.isatty() is false.
Should an issue be made for that with some kind of mock stdout or just leave it be because it's just with ides/editors that redirect output to their terminals?
hardy gorge
For me, it just runs without colors by default and with colors if I select "Emulate terminal in output console" in the run configuration
valid quest
For me nothing is shown
brazen charm
win?
valid quest
Not with pycharm and not with colored logs
Nope
We talked about this before
I think
hardy gorge
It's just been merged
brazen charm
I think colorama can't do the proper calls when the terminal is emulated through pycharm
brazen charm
F4zi have you tried emulating terminal like Ves?
brazen charm
in the run config on the top right
hardy gorge
For me it's here:
valid quest
Yeah same
cold moon
Should help.py go under evergreen (SeasonalBot)?
hardy gorge
Without it, it will just default to the old black-and-white, with it enabled, it shows colours
valid quest
Oh i got it
brazen charm
Otherwise, this is a mock for the isatty I found on SO
import logging
import coloredlogs
import sys
class PseudoTTY(object):
def __init__(self, underlying):
self.__underlying = underlying
def __getattr__(self, name):
return getattr(self.__underlying, name)
def isatty(self):
return True
sys.stdout = PseudoTTY(sys.stdout)
coloredlogs.install(stream=sys.stdout, fmt="%(asctime)s | %(name)s | %(levelname)s | %(message)s")
logging.warning("Hey")
@cold moon That's probably core functionality not bound to a season (we're mostly removing seasons anyway)
valid quest
Doesn't pycharm support colored logs as a feature?
brazen charm
That'll make coloredlogs use the colored formatter even without emulating it
cold moon
Huh, where this should go then? To same directory than bot.py?
valid quest
Btw, is there an event or something dispatched when a log is called?
tranquil topaz
is there a sure fire way to reproduce this? i cant get it to come out in anything but colours
brazen charm
run it as a script so the output goes to Python Console without emulating the terminal
No idea why the demo works but emulating terminal doesn't on win though
brazen charm
wait... it can also go to Run but I got confused out of your screenshot, but same behaviour
tranquil topaz
well.. pycharm issues are maybe a bit out of scope?
brazen charm
Yeah seems a bit out of scope for the bot directly and only affects win with not working completely, but could have a mention somewhere
green oriole
(The new resources page is looking awesome!)
tranquil topaz
well.. where do you stop then? like it does not work with sublime Text either.
yes.. the new page is very nice looking
could this be a fix with colorlogs? maybe we could write a PR to them and have them merge it.
then we did not have to do any tricks or hacks or mentions
hardy gorge
If it's still usable on Windows, just without colored logs, I don't really see an issue with the current merged PR
It's basically just reverting to what we always had in that case
I don't really see a way for us to do this cross-platform, cross-IDE
brazen charm
It's a side effect of the outputs being redirected, and the humanfriendly package returning False because of it in their terminal_supports_colors
green oriole
It does work in conhost though
brazen charm
Throwing the above into sitecustomize seems to fix it nicely
green oriole
Guys, I made a thing
green oriole
Creating channels as listed in the config file, creating a config file and update it with the new IDs
Because I was too lazy to recreate all the channels myself
green oriole
It isn't recreating the same channels, they were there only for demo purposes
eternal owl
I was too lazy to create channels and update my config file, so I made couple channels and added one channel ID for all fields in the config file
green oriole
If a core dev here is interested in putting it on the repo or somewhere more accessible
Yeah, here it is a full sync
crude gyro
interesting.
what if some of the channels already exist. would it delete them all?
@green oriole
green oriole
For now yes
I guess that's pretty easy to ignore
Done @crude gyro, now if the channel already exists, it just use its ID in the config
glass pecan
Feel free to keep working on it @green oriole.
There's been on the backburner this type of thing for quite a while: automatic setup of a test server which creates new channels that aren't found, or uses the ids of found ones, creates the appropriate emojis if they don't already exist, same with webhooks and then generating an appropriate config using the new or found values.
categories may need to be considered
glass pecan
yeah that's why the idea was part of a huge config rewrite
the current config setup isn't perfect and we had some ideas to improve it
using a single config file that only contains the items required, being able to use env vars for any of the values instead of requiring us to use the !ENV tag in the config file, things like this
it would involve a bit of extra documentation too to help with making sense of config values, options and ways to use the setup capabilities
at any rate, feel free to make a branch with your changes. it's probably a good way to kick things off regarding the concept
glass pecan
yeah you'll want to change the on_message to a command so things are structured properly
but otherwise it should transfer over to it's own extension without much issue
then you can just conditionally load the extension if it's in debug mode
i'll try spare some time with you to cleanup some of the code and help you
brazen charm
I made the config_verifier cog that checks channel constants so far. Would this make sense to have there (maybe renamed)? also noticed some kind of validating in Mark's help channel cog that could be moved there if feasible
glass pecan
yeah probably, but there's no rush
this type of thing isn't likely to be a quickly developed feature
and it's not super required so there's no need to push akarys into doing a heap all of a sudden lol
brazen charm
Not pushing, just that a new cog where it could fit exists
glass pecan
merged, maybe
fit as it is, probably not
the config setup is likely to be tweaked, so a large portion of this may not even be an extension
hardy gorge
@brazen charm Re: unit tests. If you have questions while working on that issue, feel free to ping me. We can probably work it out together. I think some of the existing tests already have most of the ingredients you'll need while writing tests for !hush.
cold moon
For SeasonalBot's .help command, should this have 1 cog per page or multiple?
cold moon
Yes, I found this. May I use some parts of this code or I have to write everything my own?
woeful thorn
There's no requirement to reinvent the wheel
cold moon
ok
cold moon
Python bot have perfect help, I can't anything that I can do better. So I can copy and paste this code and make small changes (some constants, check) and use this? (I'll reference this in commit)
hardy gorge
Sure, there's no need to reimplement it just because it already exists elsewhere. I think it's a good idea to add a comment in the file stating the origin as well; that may make it more evident to a reader that it's adapted from the other repo. If someone finds a bug, we'll need to make sure that we fix it in both repos/bots.
cold moon
OK, one question: Should I add help cog to same directory than seasons manager?
brazen charm
There is a PR open for changing the help on @stable mountain so consider looking into that
hardy gorge
I don't think a help command belongs in the seasons subdirectly. It's a core function of the bot that should probably go in the main bot folder or a logical, new subdirectory of that bot folder with a name indicating core functionality. However, moving a file should not be hard work, so using the main bot folder for now should be fine and we can always move the file later if more people weigh in.
Ah, yes, that PR is a good point.
green oriole
Is this emoji on an emoji server ? :defcondisabled:
hardy gorge
It sure is
green oriole
Okay, so we want it as an emoji directly on the dev server right?
And.. Most of them aren't on the branding repo
hardy gorge
that could be resolved
green oriole
I guess I could just upload them
cold moon
I think I'll not use this PR, due changes is still requested. This would be merged to there when PR get merged.
cold moon
OK, done now. PR maybe in some hours or tomorrow, depends how much time I have
brazen charm
How are the roles set up here? For the hush, would it be enough to override sending messages for Developers in the channel it was invoked in?
hardy gorge
That should be enough, yeah. Every public channel has Developers permissions set-up. Staff channels probably won't have that, so it should be able to handle that to stop it from throwing an exception if someone accidentally uses it there.
brazen charm
When the bot gets shut off, do the cog unloads execute? If it's worth implementing some alert logic there
woeful thorn
Alerts for what?
brazen charm
For the hush command, if some are left when it stops (won't get cancelled automatically)
woeful thorn
Is there a reason not to persist them like other scheduled tasks are?
brazen charm
Just going off of what the issue requests
A second command, !unhush, should undo the hush immediately. This is important in case the bot restarts in the middle of the duration, since we're not persisting the duration anywhere.
molten bough
I had not heard of grabify, that's a potentially pretty important commit there
crude gyro
@cold moon see this issue when you get a chance: https://github.com/python-discord/bot/issues/767
rocky bloom
Hey uh - i posted my first PR under the new template system so sorry in advance if it's not perfect
@ me if you want me to make any changes and I will
brazen charm
Something with the new default event loop seems to be broken (for me) on win after I updated to 3.8, getting
Traceback (most recent call last):
File "C:\Users\Numerlor\.virtualenvs\bot-K_QdelnW\lib\site-packages\pycares\__init__.py", line 91, in _sock_state_cb
sock_state_cb(socket_fd, readable, writable)
File "C:\Users\Numerlor\.virtualenvs\bot-K_QdelnW\lib\site-packages\aiodns\__init__.py", line 106, in _sock_state_cb
self.loop.add_reader(fd, self._handle_event, fd, READ)
File "C:\Users\Numerlor\AppData\Local\Programs\Python\Python38\Lib\asyncio\events.py", line 501, in add_reader
raise NotImplementedError
NotImplementedError
on bot.run. Setting the asyncio event loop to SelectorEventLoop fixes it
patent pivot
the bot is not running on 3.8 yet
nevermind it is
hm
well
!int e ```py
import sys
print(sys.version)
stable mountainBOT
In [2]: import sys
...: print(sys.version)
...:
3.8.2 (default, Feb 26 2020, 15:09:34)
[GCC 8.3.0]
it seems to be runnning for us
brazen charm
discord seems to be using some unix only signals now from what I understood in the notes
patent pivot
have you got the new dependency changes?
brazen charm
rebuilt the environment
patent pivot
where did you get the unix only signals stuff?
https://discordpy.readthedocs.io/en/latest/whats_new.html#v1-3-2 this is the version change made
brazen charm
ah nvm skipped on some code when I looked through it the second time
but crashes around its loop.run_forever on the default event loop
molten bough
That looks like an aiodns issue
pycares is a wrapper for c-ares, which I believe is a DNS resolution library
Solution seems to be here
This might slow the bot down slightly on windows but that doesn't really matter since it's on Linux in prod
@brazen charm (in case you didn't see it)
brazen charm
loop = asyncio.SelectorEventLoop()
asyncio.set_event_loop(loop)
I got it working with this in the init.py, but that looks like a more proper solution
Wanted to make a draft today but that kinda slowed me down
molten bough
Yeah, use the policy option so it does select a more appropriate loop otherwise
rocky bloom
we got rid of seasonalbot-chat?
woeful thorn
Yes
rocky bloom
ah
clever wraith
Hey @tawdry vapor I was pondering a bit about the reminder systems.
I've seen 2 implementations of reminder system, and they each have their pros and cons.
- the Python Bot implementation is to use the database as a backup to reschedule tasks when the bot crashes
The pros are the reminders are called exactly on time, not at any specific interval, the cons are that some tasks realistically speaking don't need to be sheduled (e.g. 2 years) and apparently asyncio.sleep is only reliable up to 48 days.
- another implementation is the use the database as a catalog of tasks and periodically check whether to call them or not.
The pros are the less processing power, but the cons are that tasks aren't going to actually be scheduled on time.
I'm trying to decided which one / whether there's an even better way for my situation.
The main purpose is to mute a user for, in general, about 2 months, but it will also be used for a simple reminder system as well.
I'm also not sure which one is easier and simpler to implement, as I'm not exactly the best at handling time in python
tawdry vapor
Pretty sure polling will be simpler to implement. I doubt processing power really is significant unless you're dealing with enormous amounts of tasks. Feel free to benchmark this yourself. If you're concerned about the 48 day thing you can re-schedule all tasks every 48 days or even poll the database every 48 days to only schedule tasks which are shorter than 48 days.
For future reference, this is getting off topic for this channel since it's no longer about our projects but rather yours.
green oriole
@patent pivot snekbox is running on 3.8, but the bot is running on 3.7
How does seasonal commands that send a message into the deprecated #seasonalbot-chat now works?
Ah okay ELA opened a PR
hardy gorge
@green oriole @patent pivot was using internal eval (!int e) not snekbox
So, the output did show the version of @stable mountain, which is now Python 3.8.2
!e
import sys
print(sys.version)
@hardy gorge :white_check_mark: Your eval job has completed with return code 0.
001 | 3.8.0 (default, Nov 23 2019, 05:49:00)
002 | [GCC 8.3.0]
!int e
import sys
print(sys.version)
In [8]: import sys
...: print(sys.version)
...:
3.8.2 (default, Feb 26 2020, 15:09:34)
[GCC 8.3.0]
So, snekbox is on 3.8.0 and @stable mountain is on 3.8.2
true fog
Sweet.
green oriole
Ah nice
Time to rebuild the venv
By the way, I thought about the dev server autoconfigurator, and I had another idea
If we want to keep the channel ordering and subcategories, the role colors, channels descriptions and everything that isn't in the config, we'd need some additional data
That's why I thought about making @stable mountain create a snapshot of the server structure containing all important information and display it on a public endpoint on the site
And the local autoconfig cog would be able to pull the snapshot, and apply it
What would you say about that?
We could also keep an history of the snapshot in case we want to switch a dev server to a previous version for some reason, because a PR was on a previous config for instance
green oriole
What do you think about this solution?
crude gyro
hm, that does sound like a good solution if our intention was to keep it in perfect sync
it doesn't sound too hard to do either
I guess I like it.
hardy gorge
Did you think about how you're planning to keep this snapshot in sync with the actual guild?
One of the reasons I'm asking that is because our new help channel system, if introduced, would cause frequent changes in the channel lay-out of the guild that do not necessarily have to be captured by such a snapshot.
green oriole
My goal was to have a command to use to trigger the sync
But we could also trigger a sync at each bot reboot for example
crude gyro
I think it'd be better to sync manually whenever needed
a command sounds fine
and I don't think that we need to update that snapshot that often, either
but that should be automated
maybe on-boot
could be part of the sync cog, I guess
green oriole
I think we could simply upload a new snapshot everytime a change is made to the server
crude gyro
yeah, but that's very incompatible with the new help channel system, @green oriole
channels will be changing order all the time
crude gyro
we could perhaps completely ignore channel order
and trigger on other changes
also yes, we should ignore archived channels.
green oriole
Or we could also ignore the help channels
crude gyro
the help channels are important, so no.
that help system should work on the test server
besides, ignoring 50 specific channels sounds like maintenance
easier to just ignore channel order changes
green oriole
I mean not trigger a new snapshot when a change is made to help channels
So the order will update only when another change is made?
crude gyro
yeah, I don't like it, why ignore all changes to help channels when we can just ignore channel order in general? why would we care about channel order on test servers?
green oriole
Because we want to really reflect the real server
crude gyro
yes and we will still do that in all the snapshots
but there's no need to update the snapshot just because we changed channel order
green oriole
Yeah, that's fair
hardy gorge
That depends on whether or not we're interested in channel categories
Because that's the thing we're going to be changing for those help channels
They'll be moved between three different categories
I don't think capturing the current state of the help channel system is important
crude gyro
any channel move should not trigger a new snapshot imo
green oriole
Yeah
hardy gorge
Whether or not there are four in the in use or five incidentally during the snapshot should not matter
crude gyro
it makes no difference. and that channel move will be reflected in the next snapshot, triggered by a change that does matter
green oriole
If it is an important reordering you could always trigger a manual snapshot
crude gyro
yes, although I cannot think of a situation where a channel reordering is important
hardy gorge
We don't follow the channel ordering here in our dev test server
crude gyro
but, hmm, permissions will also be changing on those help channels as they move between categories...
hardy gorge
The channel categories will be important, since the help channel system relies on the categories
yes
crude gyro
which isn't a channel move change, and would then trigger a new snapshot
so perhaps it's not enough to just ignore channel moves
we'd have to also ignore the repercussions of that move
hardy gorge
I'm not convinced that we need snap shots that should be kept in sync in real time
crude gyro
yeah, me neither
a daily snapshot would be fine, imo
even one for every bot restart
hardy gorge
If something affects the bot, the bot will need to be restarted anyway, otherwise the change is not relevant for the bot code/dev
so, sync on boot sounds fine to me
Which also means that just capturing the incidental state of the help channels at the moment of sync is fine
crude gyro
yeah exactly
hardy gorge
The help channel system should be able to handle any start situation anyway
since the bot can always restart
green oriole
Last time you removed the seasonalbot channels, the bot didn't restarted for instance
hardy gorge
yes, but the channel was not actually removed
green oriole
And I don't think we should sync permissions, since it would make testing messy
Ah, they are archived right
hardy gorge
That would have cause breakage since seasonalbot still uses the channel constants
green oriole
But we shouldn't ignore move to archived though
Okay yeah, we shouldn't make snapshots at any change
hardy gorge
The main point is that if something in our channel set-up changes that affect the bot directly, the bot code should have been updated to reflect that anyway
It wouldn't be good for us to make changes to the guild that would break code without changing the code
Hence, an important guild change should only happen combined with a code change and, thus, a redeployment/restart
green oriole
Okay make sense
crude gyro
@green oriole left you another review on https://github.com/python-discord/bot/pull/617
should be ready to merge soon, that one
maybe we can even merge it today with some minor changes.
brazen charm
Looks like flake8 doesn't work nicely with the walrus. https://paste.fuelrats.com/kedoburipo.py pyflakes has their error fixed on master https://github.com/PyCQA/pyflakes/pull/457
hardy gorge
I knew there was an error in pycodestyle that was fixed on master but not yet pushed to PyPI
Was there an issue in pyflakes as well?
brazen charm
pyflakes doesn't have the walrus node so it stops the whole check with an exception
green oriole
aroundfuckery haha, thanks mark
crude gyro
mark?
crude gyro
@tawdry vapor
https://tenor.com/xKcE.gif
hardy gorge
@brazen charm pyflakes stops because of a bug in pycodestyle. If you update pycodestyle by using the master version, it will work happily
It's documented in this issue (which refers you to a pycodestyle issue): https://github.com/PyCQA/pyflakes/issues/481
It's fixed on the master of pycodestyle, but not in the version on PyPI and flake8 hasn't seen a new PyPI release in a long time either
brazen charm
pyflakes crashes with "pyflakes" failed during execution due to "'FlakesChecker' object has no attribute 'NAMEDEXPR'", which is separate from the lint errors pycodestyle gives and it stops the lint (can't be ignored)
green oriole
@stable mountain is now running 3.8 right?
hardy gorge
@brazen charm Yes, that's the error I had as well and it was fixed by updating pycodestyle pyflakes
It's also the error in the issue I linked "pyflakes" failed during execution due to "'FlakesChecker' object has no attribute 'NAMEDEXPR'"
brazen charm
getting pycodestyle doesn't seem to do anything without also getting pyflakes
(except the Exxx errors ofc)
hardy gorge
This bug, that misleadingly prints that it has something to do with pyflakes, is fixed by updating pycodestyle to the latest version of pycodestyle on the git master
brazen charm
hardy gorge
The bug that causes this error to be printed: ""pyflakes" failed during execution due to "'FlakesChecker' object has no attribute 'NAMEDEXPR'"
is fixed by updating pyflakes pycodetyle to the latest version on the master of pycodestyle pyflakes
I don't really know any other way of saying that I'm a fool
brazen charm
but anyway doesn't really matter, should I make an issue so it gets fixed until a release on the bot or just avoid walrus for now?
hardy gorge
I'm not sure if we want that
So, yeah, that means no walrus for now
Even if you fix it (which I've done locally) by updating pycodestyle, flake8 will run but give linting errors for the walrus
Since flake8 thinks what there should be spacing after the :, not before the :, and it doesn't understand the := at all
it does no longer crash, though
I got around it in my advent of code repo by manually including specific noqa's for all my walrus lines
which is not nice
molten bough
I'm kind of surprised it's taken this long for tools to start supporting it
green oriole
So we can now use the walrus right?
hardy gorge
No
pycodestyle breaks on it, passing wrong values to pyflakes, causing flake8 to meltdown
hardy gorge
I'm kind of surprised it's taken this long for tools to start supporting it
@molten bough
I think there are just not enough contributing to flake8, pycodestyle, and similar tools at the moment
molten bough
Yeah, you might be right
hardy gorge
While the initial issue is fixed (but not pushed to PyPI) that cause flake8 to crash, it still doesn't allow the walrus
off
getting pycodestyle doesn't seem to do anything without also getting pyflakes
@brazen charm
yes, you're right. i was absolutely wrong. I read that issue wrong and misremembered it completely
By upgrading pyflakes to master you can fix it
Still, it has the same consequence: Installing something from GitHub master and it still doesn't fix flake8/pycodestyle not supporting it
support may come soon: https://github.com/PyCQA/pycodestyle/issues/911
brazen charm
pyflakes fixes the error, pycodestyle the lint errors so it'd work if both were from github but yeah not really best to rely on the master
hardy gorge
Yeah, I should read instead of being stubborn
If that release of pycodestyle goes through and pyflakes also does a release, we will get our support
There's currently also no support for positional-argument only syntax in pycodestyel
It's the open issue mentioned in that release issue
eternal owl
rip #seasonalbot-chat
green oriole
For the best IMO
clever wraith
where would be the best place to implement this? don't want to pollute __init__.py
woeful thorn
It was added to bot as a new cog, which should be fine: https://github.com/python-discord/bot/blob/master/bot/cogs/config_verifier.py
woeful thorn
Well, on bot we can use the YAML, which doesn't exist with seasonalbot
It does probably make sense to port over the wait_until_guild_available helper though, so we can make sure the channel cache is populated
cold moon
Can I now work with https://github.com/python-discord/bot/issues/562 ? Due moving to Py3.8 is done now.
tawdry vapor
Yes
green oriole
Hmm, there is no PATCH methods for deleted messages, should I just delete it and recreate a new one when we need to reschedule them?
The other PR has been already merged, I can't edit it anymore
cold moon
How to make my fork master branch up-to-date with python-discord's one?
brazen charm
green oriole
Add the pydis repo as a remote (most of the time upstream)
Then git pull upstream/master
eternal owl
how is git pull upstream/master different from git merge upstream/master?
green oriole
It does fetch it before
eternal owl
ooh, saves me 1 command
green oriole
Yep!
tawdry vapor
@rocky bloom For future reference, please split your changes into multiple commits. An "addressing reviews" type of commit with a bunch of unrelated changes is not good commit practice.
rocky bloom
sorry :/
cold moon
Why I'm getting sys:1: RuntimeWarning: coroutine 'APIClient._create_session' was never awaited (I'm writing tests)?
hardy gorge
Because somewhere a coroutine object is created by calling a coroutine, but it's not called anywhere.
Can you show me the code that's causing it?
cold moon
class ModerationUtilsTests(unittest.IsolatedAsyncioTestCase):
"""Tests Moderation utils."""
def setUp(self):
self.bot = MockBot()
self.member = MockMember(id=1234)
self.ctx = MockContext(bot=self.bot, author=self.member)
self.bot.api_client.get = AsyncMock()
async def test_user_has_active_infraction(self):
"""Test does `has_active_infraction` return correct result."""
self.bot.api_client.get.return_value = [{
"id": 1,
"inserted_at": "2018-11-22T07:24:06.132307Z",
"expires_at": "5018-11-20T15:52:00Z",
"active": True,
"user": 1234,
"actor": 1234,
"type": "ban",
"reason": "Test",
"hidden": False
}]
self.assertTrue(await has_active_infraction(self.ctx, self.member, "ban"))
self.bot.api_client.get.return_value = []
self.assertFalse(await has_active_infraction(self.ctx, self.member, "ban"))
My current code
green oriole
Do you have the same warning on other tests or only on this one?
cold moon
For every test that I run individually
Also multiple warnings:
/Users/user/PycharmProjects/bot-mod-utils-tests/bot/bot.py:24: DeprecationWarning: The object should be created from async function
resolver=aiohttp.AsyncResolver(),
/Users/user/.local/share/virtualenvs/bot-mod-utils-tests-lo6WvmsC/lib/python3.8/site-packages/aiohttp/connector.py:727: DeprecationWarning: The object should be created from async function
super().__init__(keepalive_timeout=keepalive_timeout,
/Users/user/PycharmProjects/bot-mod-utils-tests/bot/api.py:53: DeprecationWarning: The loop argument is deprecated since Python 3.8, and scheduled for removal in Python 3.10.
self._ready = asyncio.Event(loop=loop)
I have the first two ones, the last one is new since the migration
hardy gorge
You can ignore those for now, they're being worked on. They are deprecation warnings.
some are outside of our control (upstream needs to update)
cold moon
Yeah, but should I ignore this sys:1: RuntimeWarning: coroutine 'APIClient._create_session' was never awaited too?
green oriole
You patched APIClient somewhere?
cold moon
All my code is in this chat
hardy gorge
No, that one should not be ignored; can you paste your full file in the paste server? I'd like to run it myself. I think I know what's happening, but debugging at a distance is always a bit more complicated that being able to experiment a bit yourself.
hardy gorge
Why?
green oriole
No, no it doesn’t
green oriole
We should find out where this function is called instead I think
As far as I understand, this function shouldn’t be called at all during tests, since everything should be mocked
hardy gorge
@cold moon I can't reproduce the problem locally; your test runs fine on my end
✔ ~/pydis/repositories/bot [master|⚑ 3]
15:49 $ pipenv run test
Loading .env environment variables…
2020-03-03 16:02:27 | bot.constants | INFO | Found `config.yml` file, loading constants from it.
2020-03-03 16:02:28 | deepdiff.diff | INFO | jsonpickle is not installed. The to_json_pickle and from_json_pickle functions will not work.If you dont need those functions, there is nothing to do.
/home/sebastiaan/pydis/repositories/bot/bot/bot.py:24: DeprecationWarning: The object should be created from async function
resolver=aiohttp.AsyncResolver(),
/home/sebastiaan/.local/share/virtualenvs/bot-I8fh9oki/lib/python3.8/site-packages/aiohttp/connector.py:727: DeprecationWarning: The object should be created from async function
super().__init__(keepalive_timeout=keepalive_timeout,
/home/sebastiaan/pydis/repositories/bot/bot/api.py:53: DeprecationWarning: The loop argument is deprecated since Python 3.8, and scheduled for removal in Python 3.10.
self._ready = asyncio.Event(loop=loop)
..................................................................................................................................................................sssssssssssssss..........................................................................
----------------------------------------------------------------------
Ran 237 tests in 6.065s
OK (skipped=15)
That's the full output
green oriole
It does skip for me too
hardy gorge
That's normal
green oriole
I think it is normal
mellow hare
Gotcha
hardy gorge
Those are tests that are not yet implemented
hmm
I get it now
I'll investigate what's up
It's not something that happens with your test
It happens when I run a test file in isolation; it does not matter which
So, you're not doing anything wrong and I'm going to go on a wild goose chase
I think I know what it is
cold moon
May this problem when I don't have config.yml file? I have .env but not config for this.
green oriole
It shouldn’t
hardy gorge
It's the self.recreate() call in the __init__ of the actual APIClient it seems
I'm not sure yet why it runs
Ojh
Oh
I know
hm
This is going to be a nice issue
@cold moon Don't worry about it for now. I'm going to look into it when I have time. I know what the issue is, but I'm not sure what the way to resolve it is yet
Thanks for bringing it up
cold moon
ok, I'll continue writing tests
hardy gorge
Can I give you one piece of advice upfront?
cold moon
What?
hardy gorge
In the test you have above, you actually test two things; the second test will never run if the first fails.
It's better to make sure that the tests are truly independent of each other
cold moon
Oh, so different tests?
hardy gorge
Also, if a part of the implementation changes, then the first call could influence the second call, because you use mutable mock objects
Yes or use self.subTest, depending on the level of independence you need to guarantee
green oriole
I’m curious, what do you think the issue is?
hardy gorge
The "issue" is that we changed how we create a session. I create an instance of Bot in tests.helpers to be able to create a realistic MockBot with strict spec_set attribute restrictions. However, the __init__ method of our Bot creates an instance of the actual APIClient, which in turn in its __init__ calls the recreate method that does this:
def recreate(self) -> None:
"""Schedule the aiohttp session to be created if it's been closed."""
if self.session is None or self.session.closed:
# Don't schedule a task if one is already in progress.
if self._creation_task is None or self._creation_task.done():
self._creation_task = self.loop.create_task(self._create_session())
So, it then creates a coroutine object to be scheduled in the last line, but that one never goes anywhere.
hardy gorge
that's when the warning will happen
When a coroutine object is deallocated, it will check if it has been awaited; if not, it will give you that RuntimeWarning
green oriole
Ah, so the issue is the opposite
hardy gorge
Okay, this is literally the most dirty trick ever, but it does guarantee that we'll actually have an object for the spec_set with all the attributes.
The "issue" is that discord.py sets a lot of attributes dynamically, which means that you'll need an actual instance to get it right
with unittest.mock.patch("bot.bot.api.APIClient"):
bot_instance = Bot(command_prefix=unittest.mock.MagicMock())
Anyway, I'll need to test that and figure out if there's a better option
cold moon
Bruh, how should I test send_private_embed?
hardy gorge
test if it behaves correctly in all situations
if you set side_effects for user.send you can trigger exceptions when it's called in the function
That will allow you to test the except clause
cold moon
OK, thanks
cold moon
But for notify_infraction, is there any way to get embed from this for analyzing?
tawdry vapor
Mock send_private_embed and look at its call_args
cold moon
Can anyone help me? I need currently go to music school, but for one of my tests, I need test cases. I made one myself:
test_cases = [
{
"args": (self.user, "ban", "2020-02-26 09:20 (23 hours and 59 minutes)"),
"expected_output": {
"title": "Please review our rules over at https://pythondiscord.com/pages/rules",
"colour": Colours.soft_red,
"author_name": "Infraction information",
"author_url": "https://pythondiscord.com/pages/rules",
"url": "https://pythondiscord.com/pages/rules",
"footer": "To appeal this infraction, send an e-mail to appeals@pythondiscord.com",
"icon": Icons.token_removed,
"description": textwrap.dedent("""
**Type:** Ban
**Expires:** 2020-02-26 09:20 (23 hours and 59 minutes)
**Reason:** No reason provided.
""")
}
}
]
This is for notify_infraction in moderation utils. Can anyone make different cases for test?
brazen charm
nobody's rushing you to do it
cold moon
ok
woeful thorn
If we're going to have this amount of specificity on the test then it would be a better idea to also mock the URLs and email addresses so we don't have to change the tests every time one of them changes
mellow hare
Is there a way to force a pipenv venv to go from one Python version to another? Or is it better to just trash the venv and make a fresh one
I feel like it's the latter but I couldn't remember
molten bough
Trash it
mellow hare
Fair enough
molten bough
it's too much work to try to move it tbh
and some libs will need recompiling on linux
mellow hare
Yeah I remember trying it before and it was a dumpster fire
But I wasn't sure if something had changed since then
green oriole
You may also need to rebuild the docker image
hardy gorge
There's no real reason to attempt to migrate your venv (if it's even possible), unless you want to save time on that endless locking. (Thanks phone, it almost 'corrected' that to licking.)
hardy gorge
That is true
mellow hare
Having to re-learn so much so I can contribute to the repos again
Attempting to teach myself unittesting and mock and what have you
green oriole
We have some link at the bottom of the readme
The real python tutorials are really good
valid quest
Yeah
brazen charm
Mostly done with the base commands in https://github.com/python-discord/bot/pull/812 if anyone could take a quick gander if that's how it was envisioned
mellow hare
With the current implementation there doesn't seem to be a way to make the silence be indefinite
..... he says only just now looking at the converter
Sorry
brazen charm
Did you look at the command help? To know if it should be clearer there
mellow hare
Yeah it's a smidge unclear. I'd specify that adding "forever" as the duration will make the channel silenced until it is !unsilence'd
Or something like that
Something to better elaborate that forever is the keyword
You'll also want to expand on what the defaults are
woeful thorn
I don't think we really need a forever
mellow hare
Current channel will be silenced unless one is specified
I could see someone forgetting
-coughmecough-
woeful thorn
I know it's mentioned in the issue but I don't see a scenario where we'd want to indefinitely silence a channel
mellow hare
And if we really really needed to, there'd be nothing stopping us from putting it at like 99 hours or something
brazen charm
there is a 15 min limit on timed
mellow hare
Ah gotcha
That should also be in the help
I also wonder if we shouldn't just use the Duration class for that. Or at least inherit from it
I'm just thinking out loud mainly
I haven't tested it
brazen charm
No reason to drag datetime into it and would need to restrict it for the minutes, but maybe a subclass could be useful if keeping check on the timeouts is wanted ( instead of going through time.monotonic or something); asked about that on the issue
(the record keeping)
cold moon
https://github.com/python-discord/seasonalbot/pull/361#discussion_r387451579 who should have ability to refresh genres?
gusty sonnet
You can impose a cooldown per user, and let any mod/dev to do it anytime
cold moon
I can't find Greedy anywhere.
gusty sonnet
You can read about it here @cold moon - https://discordpy.readthedocs.io/en/latest/ext/commands/commands.html#greedy
I haven't tested with str however, since by default they are strings, so it may not work when you want the last one to be an int. Do give it a try!
In the worst case you can create a new Converter that raise error when it's an int, but that'll be a lot more work than parsing the arguments manually ( split() and get the last argument )
cold moon
@gusty sonnet I can't user Greedy. dpy docs:
To help aid with some parsing ambiguities, str, None and Greedy are forbidden as parameters for the Greedy converter.
gusty sonnet
Yep so you need a custom Converter.
Then I suggest trying to parse manually then
glass pecan
@gusty sonnet it's not impossible, you just need to quote it.
if you don't want to quote it, make the genre a consume-rest argument and the amount an Optional
it's not really easy to process arguments like this otherwise, and you're left with some ugly manual processing
cold moon
So, I should use *args, split them, check does last item isdigit, if is, storage and remove it and then join others back to one?
glass pecan
no, that's part of the ugly manually processing way
async def games(self, ctx, amount: typing.Optional[int] = 5, *, genre: str = None):
...
!games visual novel # will work
!games 10 visual novel # will also work
note: i have not looked at your code yet
cold moon
Ok, thanks
glass pecan
seems you have genre as optional also
so you'd just give it a default i think in the above example
edited to be default None
cold moon
Yes, this works now. I'll fix others too now and then I'll push
cold moon
I have one problem with fuzzywuzzy: I get TypeError: expected string or bytes-like object.
My code:
possibilities = "`, `".join(process.extractBests(genre, self.genres, scorer=fuzz.ratio, score_cutoff=90))
(I used code from Python !help command command matcher)
glass pecan
i don't mind that you've used string templating, but why the usage of string.Template explicitly over the newer str.format templates?
Also the style of multiline strings you've got is inconsistent to typical project style.
Example, you currently have:
GAMES_LIST_BODY = Template(
textwrap.dedent("""
fields cover.image_id, first_release_date, total_rating, name, storyline, url, platforms.name, status,
involved_companies.company.name, summary, age_ratings.category, age_ratings.rating, total_rating_count;
${sort} ${limit} ${offset} ${genre} ${additional}
""")
)
body = GAMES_LIST_BODY.substitute(params)
What I'd typically expect is:
GAMES_LIST_BODY = (
"fields cover.image_id, first_release_date, total_rating, name, storyline, url, platforms.name, status, "
"involved_companies.company.name, summary, age_ratings.category, age_ratings.rating, total_rating_count; "
"{sort} {limit} {offset} {genre} {additional}"
)
body = GAMES_LIST_BODY.format(**params)
as this is using standard inbuilt methods, there's then no need to import string and textwrap
cold moon
Ok
glass pecan
KeyError: 'Rg' 
glass pecan
self.genres[genre] seems pretty fragile
cold moon
I use except KeyError
glass pecan
you're retrieving all genres from the beginning right?
and then it's just cached
you're defining self.genres outside of init also, which isn't really optimal
it's not clear it exists as an instance variable until you dig further into the code, when it should be clear within __init__
cold moon
OK, I'll move this to init
glass pecan
you don't need to move the value assignment, just be sure to add a self.genres = None or something
however i also don't know what types are in this structure
is self.genres containing a Dict[str, int]
like ```py
{"rpg": 5}
or is it something else
cold moon
Yes, this is str,int
glass pecan
then it might be worth adding that in the definition in init as a good form of self-documentation, so we know how to interact with it
glass pecan
yeah does it raise only when you typo
cold moon
Ah, I don't need fuzzywuzzy. I solved this:
possibilities = "', '".join(difflib.get_close_matches(genre, self.genres, cutoff=0.4))
glass pecan
ok. this is only for the suggested close matches right?
gusty sonnet
As a QoL, if there is only one close match I'd consider using that, and so .game hack works just as fine, and won't require users to type the full .game hack-and-slash for it to work
glass pecan
using that
using what
cold moon
I can add Hack as a alias of hack-and-slash
glass pecan
lets not add a million aliases lol
cold moon
okay
glass pecan
shira just really likes partial matches by the sounds of it
cold moon
I'll now replace string.Template to .format() and then I push
gusty sonnet
I dont really want to .game hack # Not found do you mean hack-and-slash .game slash # Not found do you mean hack-and-slash .game hack-and # Not found do you mean hack-and-slashAnd looks like difflib.get_close_matches(genre, self.genres, cutoff=0.4) returns an Iterable, so you can probably check its length ( cast to tuple / list if need be )
If there is only one match, might as well help the user
glass pecan
right, i understand now. sorry, the wording was ambiguous so had no idea what to apply "that" to
gusty sonnet
Yeah sorry, I was referring to the only one close match
glass pecan
yeah makes sense
id still only do that if the score is high enough though, as you'd end up with some weird results otherwise
cold moon
OK, I'll add this too.
glass pecan
if it feels involved, you can leave it for another issue ticket for later improvements
this ticket looks like it's gone through a fair bit of reviewing, so i'm sure you'll likely just want to get to a state of mergeable
gusty sonnet
I'm mostly testing for functionalities now, I believe it is mergeable as soon as the issues are addressed, mainly the one where you cannot search for Visual novel
gusty sonnet
Yes, without quotes lol
cold moon
I'm currently stuck with one conflict: When I want this suggest like hack and slash when you use hack, I can't use auto getting games with fixed genre, due my offcut is too small for 1 item. What should I do?
Do I remove auto refetching with correct genre now?
(And quick question: How much contributions is required to get Contributor role? 😅)
glass pecan
contributor role has multiple pre-requisites, not just the amount of code contributed.
regarding your string matching questions, that'll be something you have to ask shira, as it's their review that has requested it
cold moon
INFRACTION_EMBED_DEFAULT.copy().description.format(**{
"type": "Warning",
"expires": "N/A",
"reason": "Test reason"
})
How should I modify this for getting embed?
gusty sonnet
To answer your question @cold moon let's take a look at get_close_matches() first
In reality, this is just calling SequenceMatcher().ratio() on each of the words in your sequence, let's test to see if it is actually doing that
!e ```py
import difflib
string = "Simulator, Tactical, Quiz/trivia, Fighting, Strategy, Adventure, Role-playing, Rpg, Shooter, Music, Indie, Turn-based-strategy, Tbs, Pinball, Puzzle, Real-time-strategy, Rts, Hack-and-slash, Visual novel, Platform, Racing, Sport, Arcade, Point-and-click"
genres = string.split(', ')
def get_top(query: str, number: int = 4) -> list:
return sorted(
((round(difflib.SequenceMatcher(None, query, genre).ratio(), 2), genre) for genre in genres),
reverse=True
)[:number]
print('hack', *get_top('hack'), sep='\n')
print('hack-and', *get_top('hack-and'), sep='\n')```
stable mountainBOT
@gusty sonnet :white_check_mark: Your eval job has completed with return code 0.
001 | hack
002 | (0.4, 'Racing')
003 | (0.33, 'Tactical')
004 | (0.33, 'Hack-and-slash')
005 | (0.32, 'Point-and-click')
006 | hack-and
007 | (0.64, 'Hack-and-slash')
008 | (0.43, 'Racing')
009 | (0.38, 'Tactical')
010 | (0.35, 'Point-and-click')
Now let's try with get_close_matches()
!e ```py
import difflib
string = "Simulator, Tactical, Quiz/trivia, Fighting, Strategy, Adventure, Role-playing, Rpg, Shooter, Music, Indie, Turn-based-strategy, Tbs, Pinball, Puzzle, Real-time-strategy, Rts, Hack-and-slash, Visual novel, Platform, Racing, Sport, Arcade, Point-and-click"
genres = string.split(', ')
print('hack', difflib.get_close_matches('hack', genres), sep='\n')
print('hack-and', difflib.get_close_matches('hack-and', genres), sep='\n')
@gusty sonnet :white_check_mark: Your eval job has completed with return code 0.
001 | hack
002 | []
003 | hack-and
004 | ['Hack-and-slash']
This is pretty consistent with our finding, how do I know that, because in get_close_matches()
It has a kwarg cutoff default to 0.6
So anything with ratio above 0.6 will pass
Which is consistent with what we found above
Now, then, how do we apply this to hack and hack-and-slash case
cold moon
OK, so I should to do this with SequenceMatcher?
gusty sonnet
The short answer is, you split the hack-and-slash
The long answer is, yes, if you do not want a custom algorithm, you should get around SequenceMatcher
It is the core of fuzzywuzzy as well
One idea come to my mind, is to get the max ratio of the word vs each words
Here's a very crude example of what I am thinking of
!e ```py
import difflib
import re
regex_non_alphabet = re.compile(r"[^a-z0-9]", re.IGNORECASE)
string = "Simulator, Tactical, Quiz/trivia, Fighting, Strategy, Adventure, Role-playing, Rpg, Shooter, Music, Indie, Turn-based-strategy, Tbs, Pinball, Puzzle, Real-time-strategy, Rts, Hack-and-slash, Visual novel, Platform, Racing, Sport, Arcade, Point-and-click"
genres = string.split(', ')
def get_top(query: str, number: int = 4) -> list:
results = []
for genre in genres:
ratios = [difflib.SequenceMatcher(None, query, genre).ratio()]
for word in regex_non_alphabet.split(genre):
ratios.append(difflib.SequenceMatcher(None, query, word).ratio())
results.append((round(max(ratios), 2), genre))
return sorted(results, reverse=True)[:number]
print('hack', *get_top('hack'), sep='\n')
print('hack-and', *get_top('hack-and'), sep='\n')
@gusty sonnet :white_check_mark: Your eval job has completed with return code 0.
001 | hack
002 | (0.75, 'Hack-and-slash')
003 | (0.44, 'Point-and-click')
004 | (0.4, 'Racing')
005 | (0.33, 'Tactical')
006 | hack-and
007 | (0.64, 'Hack-and-slash')
008 | (0.55, 'Point-and-click')
009 | (0.43, 'Racing')
010 | (0.38, 'Tactical')
Of course this will have its downfall
I suggest playing around with this snippet, use different query and see what fits the bill for you
cold moon
OK, thanks
Maybe I should do this auto call when 1 of them have at least 0.60 ratio. When all is lower, just show possibilities
gusty sonnet
That's also one way to do it, just ask yourself, when you do .games hack and all scores are below 0.6 , what do you want it to happen
Add more test cases and try it until it feels good for you
If you are interested in it, @stable mountain has an algorithm to deal with tag name searching, it is very similar to what you are doing, but it is a custom algorithm
cold moon
When all is lower than 0.6, this would just show possibilities.
gusty sonnet
Otherwise I think your current code is very close to be mergeable and so other stuff can be added later
It is totally fine to simply reply to my conversation as "I will add an issue about this for now" is fine, I mainly want to raise the issue of spaces in genres
cold moon
I think this may currently best, due then I can move on Python's unit tests. I'll create issue about this after PR get merged, due I don't want ghost issue (issue for code that is really not in repo).
gusty sonnet
That is totally fine!
cold moon
Any feedback https://paste.pythondiscord.com/huwujequpa.py Is these test cases enough for mod utils notify_infraction?
cold moon
3 of 6 mod utils unit tests is done.
cold moon
Can anyone help me with https://paste.pythondiscord.com/imawuwaqot.py ? I'm little stuck:
AssertionError: <AsyncMock name='mock.bot.api_client.post[15 chars]504'> != [{'id': 1234, 'avatar': 'test', 'name': '[66 chars]rue}]
tawdry vapor
It uses post, not get
cold moon
Oh, thanks. This works now
tawdry vapor
Why are you awaiting post_user yourself?
cold moon
What?
tawdry vapor
result = await post_user(*args)
cold moon
With what should I replace this?
tawdry vapor
This test is for post_infraction. It should not be asserting the return value of post_user - that should have its own test(s).
eternal owl
- Replaced code which used the "os" module to use "pathlib".
- Using pathlib.read_text() method instead of using context manager to open a file and then reading it.
- the get_tags() method and _get_tag() methods are not async anymore.
- Storing all the file names in self._cache which is populated with the get_tags() method.```
is this commit message good?Are there any others issues you guys have with the reddit command? If so add em here(I think we got all of them listed)
https://github.com/python-discord/seasonalbot/issues/348
tawdry vapor
You misspelled "instead"
Also, that's the body of the commit message. What's the summary?
eternal owl
where does the summary go?
tawdry vapor
It looks like you threw in too many unrelated changes into one commit, so it will be difficult to make a summary which covers it all.
eternal owl
I am using pycharm's version control panel for the first time
tawdry vapor
The first line is the summary, then a blank line, then the body.
eternal owl
What can be a good summary for this 🤔
tawdry vapor
That's what I mean - you have too many unrelated changes.
cold moon
OK, now only 1 function left (hardest): post_infraction...
tawdry vapor
And if you want to learn more about commit messages, read this https://chris.beams.io/posts/git-commit/
eternal owl
thanks
eternal owl
is this good @tawdry vapor ?
Use "pathlib" instead of "os" module and context manager
The pathlib module simplifies opening and reading files(using Path().read_text()), hence the os module and the context manager are no longer used.```Yes. Though it's not necessary to get so specific by mentioning read_text.
eternal owl
okay
cold moon
Please help with https://paste.pythondiscord.com/itecejarim.py . Last case failing:
AssertionError: None != [{'id': 2, 'inserted_at': '2018-11-22T07:[154 chars]rue}]
I need fresh look
brazen charm
Not sure if in the issue or here... but @hardy gorge https://github.com/python-discord/bot/issues/553 mentions outdated 3.7 (and a missing word in the note ... please make sure to get ...)
cold moon
Can anyone look into code? I just can't find where is this that raise exception and return None
tawdry vapor
@cold moon reset_mock doesn't reset the return value or side effects unless you explicitly tell it to
.reset_mock(side_effect=True)
That matters cause you're only conditionally setting the side effect rather than doing it for all subtests. So the side effect ends up being carried over to the next subtest.
You could reset the side effect, or you could just set the side effect always. If you give it a value of None it's effectively resetting it anyway.
Also, there's no reason to use such accurate data for the test cases. The expected_output could be "foo" and the test would be just as effective.
brazen charm
With the passed value expected in a result of a test, is it fine for it to just be in the string... or something like this?
("abc", f'{"abc"} is not a valid minutes duration.'),
("10d", f'{"10d"} is not a valid minutes duration.'),
Sorry, I don't understand. Can you clarify passed value expected in a result of a test?
brazen charm
the converter I'm testing raises a BadArgument with the passed argument being present in the error message
e.g. !command invalid_value results in BadArgument("invalid_value is invalid")
tawdry vapor
You're trying to assert the value of the error message?
tawdry vapor
I believe msg is the output message to display when an assertion fails (similar to how all other assertions use it)
If you need to filter by the error's message contents, you need to use the regex assertion
And since you can use regex, you can just include the value at the start rather than writing out the entire message.
brazen charm
was going off of the other tests in converters but yeah from the docs it doesn't look like the msg has much purpose beyond existing
cold moon
Huh, finally is this done... tests for mod utils is done
cold moon
Huh, this is not done yet...
green oriole
How many coverage do you have?
cold moon
Coverage report show 100%
green oriole
So.. should be go?
hardy gorge
What's confusing you, @cold moon?
hardy gorge
We're not using an actual get method but a patched one that's part of the APIClientMock. It means that we should be able to assert if we called the get method with the appropriate payload (the user id and infraction type provided as arguments to the function) since that is essential for the implementation of this feature. Accidentally removing the user__id part, for instance, would mean that each time this function is run, we request the entire infractions database.
So, asserting that we're actually providing the right arguments to the get method here means that such a mistake can never make it's way into the actual code base in production.
green oriole
You need to test that every functions got the right arguments, in order to make sure that we don’t accidentally broke a feature
cold moon
Oh, now I understand
green oriole
The fact that these functions are mocked make them always return the right value during testing, but what if we didn’t supplied the right arguments?
hardy gorge
I'm not sure about every function, but this one is quite essential (and we're mocking it anyway, so it should be as simple as a reset and assert each iteration).
green oriole
I tried to assert every functions in the snekbox tests, as even if a little function is broken, all the feature goes down with it
hardy gorge
I think we should keep the level of testing reasonable; if that's a reasonable level for those functions, then it's obviously okay
However, take this utility function, it has two TRACE-level logging statements; I wouldn't test those as they are non-essential (we gain no assertion power from including them) and requiring us to test them would only serve to demotivate the developers. Mind you, if they were to be broken, the tests would fail anyway, since we'd get an uncaught exception in the test suite.
So, we don't really have to assert their contents or the call more verbosely than that
green oriole
Yeah, I didn’t asserted the log either, it wouldn’t really make sense to do so
hardy gorge
It sometimes makes sense. Say if you have an except SomeException clause with a warning/error/critical-level log, then it would be beneficial to use assertLogs to make sure we include the relevant information in the logging message.
green oriole
@gusty sonnet from my testing on my dev server, which si slower than the production environment, I always get a one second delay, did it ever happened here that they were a 5 seconds delay? It seems like a lot, since the delay is actually caused by code execution delay
Adding the padding made the tests fail haha
hardy gorge
Well, that's because the utility function returns a string
green oriole
I think we should actually refactor humanize_delta in that case, do we?
hardy gorge
Anyway, why don't we just add a rounding kwarg to humaze_delta instead?
green oriole
Yes, I corrected it
hardy gorge
There are several fairly easy methods of rounding a datetime to a unit
green oriole
I mean, the same issue is happening in the reminder cog, that directly use the humanize_delta function, so we would have to duplicate the rounding code here
Instead of rounding the output, why not directly blowing up the issue at the source
hardy gorge
I'm not sure what you mean
Are you suggesting to not change humanize_delta and instead take care of rounding at every place that calls it?
green oriole
No, the opposite
hardy gorge
I'd say adding an optional kwarg to humanize_delta with a unit to round to is far more easy to implement, maintain, and test
Right
That's what we're all suggesting then
Anyway, why don't we just add a rounding kwarg to humaze_delta instead?
@hardy gorge
green oriole
The issue is because the way humanize_delta works, I remember finding it out when I was working on the reminder cog
I mean, do you want to add a rounding mechanic to humanize_delta or change the way it works?
hardy gorge
My suggestion would be to do two thing:
- Define a help function that takes a datetime and rounds it to the provided unit
- Add an optional kwarg to humanize_delta that uses that helper function if a unit was provided
That means we can use humanize_delta with any precision we want, but still be able to round if we don't care about small fluctuations
green oriole
Okay
cold moon
Should seasonalbot get ’weather’ command, that fetch weather information by location?
glass pecan
No
molten bough
Is it actually going to remain by that name?
molten bough
Fair
crude gyro
I think the name is fine even after the deseasonification. It will still be changing our icon and banner according to what season it is, it just won't be selectively making available features.
green oriole
By the way, everyone kept the @Lovefest role, it might be worth removing it
woeful thorn
They won’t die
hardy gorge
Maybe they'll feel a bit of love
I don't really have an opinion on changing the name of seasonalbot other than that a lot of people know it by that name and there may be external links that point to it. Changing the name seems a lot of work and may break external links (if they exist), without a lot of benefit.
glass pecan
it was seasonalbot before the season loading
so i see no major reason to change either
glass pecan
I noticed a stagnant PR so I decided to quickly apply review suggestions to get it merged. If I could get some reviews on it so it can be closed, it would be appreciated:
https://github.com/python-discord/bot/pull/643
glass pecan
did you want to re-review the roles pagination one ela
you requested changes, but they've been addressed
woeful thorn
Geez dad
glass pecan
you don't have to though if you're busy, as i did it
i just didn't want to dismiss your review without asking first
feels rude
lol
glass pecan
there's a mistake lol
and yeah the fuzzy matching is annoying
we've got multiple implementations of it at this point
and i just didn't want to build one right now that's properly robust and dependable to use project-wide
woeful thorn
I don’t think a zen of python command has to be missile defense caliber, this was supposed to be a quick thing
glass pecan
yep
which is kinda why i felt it's just not worth halting the pr over it
i still love the aaaaaaaaa example tho
the perfect result returns
glass pecan
haha
i think that kinda falls under explicit is better than implicit, if you take the original intention away 😄
woeful thorn
Explicitly ping scragly for human parsing of the zen of python
glass pecan
that's probably the one "rule" i break most often
i have implicit behaviours for shitloads of stuff
the second one would probably be the only having one way to do something
i have multiple ways to do something based on the situation
¯_(ツ)_/¯
usually simple things allow for simple apis
but then i have a ton of more complex ways to do the same thing as you can control it's behaviour more
glass pecan
haha nice
hardy gorge
this is interesting
glass pecan
i guess that's like pathlib's Path
you can arg it or / it or entire string it
what's the /// turn into
glass pecan
strange situation
hardy gorge
best_match = ""
match_index = 0
best_ratio = 0
It means that none of the matches was better than the initialization
glass pecan
makes sense
cold moon
appeals@pythondiscord.com Maybe should this moved to constant in bot/cogs/moderation/utils.py?
glass pecan
why
glass pecan
it's not hard to just slip in another BadArgument for that one i think
ill do that now
cold moon
ok
hardy gorge
I wonder if just setting the default best_ratio to -1 works
that would give us another quasi-random result, just like the nonsense searches do
hardy gorge
Nah, it's just a simple comparion later on best_ratio < new_ratio or something
It's not used for anything else
I think the original thing thought that at least one line was going to match above 0
woeful thorn
Or, if we really want to have fun then just init the string to something funny
hardy gorge
There are an infinite number of inputs that give this result by the way
Just try it with any symbol not contained in any line
+zen $$$ also works
+ is my dev prefix
glass pecan
i was just going to ```py
if best_match == "":
raise BadArgument("I didn't get a match! Please try again with a different search term.")
ah
glass pecan
saves thinking
hardy gorge
that works
cold moon
Please help with https://paste.pythondiscord.com/ejuniqahib.py . result is AsyncMock, but should bool
hardy gorge
When you do not set a return value (or side-effect) for the ctx.send AsyncMock, it will return another AsyncMock after being awaited. (unittest.mock mock classes always return a "child mock" of their own type after being called/called+awaited.)
As you do:
if send_raise:
self.ctx.send.side_effect = send_raise
You only set something is send_raise is truthy
or is that not what's happening?
I don't have the code of the thing you're testing handy
Right, it's close, but not the send method
The coroutine you're testing will never call+await ctx.send since that happens in another coroutine you've mocked
However, since send_private_embed is mocked, await send_private_embed(user, embed) will return a new AsyncMock, which will then be returned by the notify_infraction coroutine
cold moon
But how should I get value? Bool
hardy gorge
what do you want await send_private_embed(user, embed) to return?
You should make sure you set that on send_private_embed_mock
(The mock created by the patch is passed to the method as an argument)
cold moon
ok
woeful thorn
How did I do what
eternal owl
did you litrelly edit the file?
woeful thorn
?
eternal owl
I mean how did you commit on a PR which you have not opened
woeful thorn
When you open a PR you allow commits from maintainers
That’s why we ask everyone to check the box
eternal owl
Ooo right
cold moon
https://paste.pythondiscord.com/mawakoyapa.py Can anyone help me again?
AssertionError: None != [{'id': 1, 'inserted_at': '2018-11-22T07:[148 chars]lse}]
hardy gorge
Can you paste the full traceback as well?
cold moon
ok
Here is full. There is some more errors https://paste.pythondiscord.com/abuhimowuz.py
Sorry about asking so much. Just after 3 hours of work, my own code is sometimes so confusing...
hardy gorge
I'll get back to you later; I have an appointment in five minutes
cold moon
Ok
hardy gorge
One thing that I can't quite make out is which test case is failling because the arguments you pass to subTest do not indentify that properly. They all have more or less the values.
cold moon
Last error is about last case: this have datetime
2nd is 3rd case
And 1st is 2nd case
hardy gorge
Hmm, okay
The last error you're getting is because when the previous test case raises an exception, it will not reset the mock
Since an AssertionError will break out of the subTest context manager
So, the mock is not reset, will raise an exception (side_effect is still there), which causes the actual coroutine to return None
mellow hare
https://github.com/python-discord/bot/issues/321 Okay, I've got a logistics question on this one. This is for automatically removing someone from the watch list if they're perma banned.
Currently we have the unwatch command, however I don't believe that just calling that command from inside a ban command would be appropriate, as it does have messages sent to the context channel and that doesn't seem like it would be useful if done in a public channel. Would it make sense then to make a separate helper function within the infractions file specifically to remove the person from the list? It doesn't feel very DRY but I'm not coming up with a decent idea off the top of my head
woeful thorn
I don't see why you can't refactor the unwatching logic into its own method that has the ability to redirect & customize the output to a different location
mellow hare
So you mean just modify the command itself and tweak it to accommodate what we need it to do?
Makes sense. I'm always wary of modifying things that currently work, I guess
woeful thorn
No, move the API interaction to its own method that the command calls
hardy gorge
I think the real complication is making sure that an unwatch only happens for permbans, not tempbans.
If someone on bb was tempbanned, I don't think the logical step is to unwatch them.
mellow hare
I was considering literally just dropping the function/method call into the ban command
Since the ban and tempban are separate
hardy gorge
We used to have an event listener, but it obviously does not discriminate as Discord has no notion of tempbanning.
mellow hare
Right, but it's less of an issue since they're separate commands
I can't really think of any specific reason we'd want the unwatch command to worry about where the response message is sent. The only instance where that would/should matter is for this ban autoremove
woeful thorn
The unwatch command doesn't, the helper would
mellow hare
Oh that would be... the post_infraction() one?
Sorry, having to refresh myself on everything
Oh no, I get you
The helper function that we would be doing for this
hardy gorge
We do not have more things that trigger on permban only right? It's trivial to dispatch an event for it ourselves, which allows you to just use a listener wherever needed and keep all the logic at the relevant place. I just don't think it's worth it for one listener.
woeful thorn
Just the modlog
I don't think this needs to be any more complicated than adding an "is_auto_unwatch" flag or something to control the behavior
mellow hare
That's about what I figured
modest otter
wait, is this channel for discord.py, or just general python project discussions?
woeful thorn
And it needs to go in 3 places, ban, shadowban, and editing an infraction to be permanent
mellow hare
Last one will likely be trickier
woeful thorn
Per the channel topic, this is for the discussions of this organization's projects
modest otter
oh, my bad. wrong channel
mellow hare
No worries
woeful thorn
Not really, there's already a conditional block for editing it to a permanent infraction
hardy gorge
I don't think this needs to be any more complicated than adding an "is_auto_unwatch" flag or something to control the behavior
No I don't think so either. I was just musing about how simple it is to dispatch an event with bot.dispatch(something, *args, **kwargs) and have an on_something event listener registered. That should be all that's needed. Not worth introducing something non-standard, though.
woeful thorn
It would certainly be cooler
mellow hare
True, would be neat
modest otter
wait so are all u guys part of one project?
mellow hare
Yep yep, anyone can work on the repos
modest otter
What is the repos?
mellow hare
@woeful thorn Do we have an example of a command that has a flag like what we were discussing?
I'm never sure how that'd work discord commands
woeful thorn
The command doesn't need the flag
You're calling the helper you're creating, not the command
mellow hare
Right got it. Sorry, I always feel like a bother
Ohhhh okay, it just actually clicked this time. It's similar to how we have the ban command but we also have the apply_ban() method
Okay, that's clicking now
mellow hare
The combined work of dozens of people over years
mellow hare
Why does that feel low
mellow hare
That sounds right
patent pivot
16855 on seasonalbot
molten bough
How're you counting that
mellow hare
Huh, that has more than I thought
patent pivot
I ran tokei, I'm gonna get some more specific stats now
okay lines of python code
bot: 14122
python-challenges: 217
seasonalbot: 7644
site: 8368
snekbox: 432
Okay, so I wasn't THAT far off on my original guess
patent pivot
calculated with ```bash
for dir in /bin/ls; do;
echo $dir: $(tokei -t=Python $dir -o json | jq ".inner.Python.code")
done
It's about what I'd expected
gusty sonnet
Imagine if we did it with discord.js
gusty sonnet
I know that I rewrote the js bot of 15k lines into a nice, maintainable 5k in python with a load more features lol
modest otter
how experienced r u guys with python
molten bough
pretty experienced
modest otter
How many years?
molten bough
11 years myself
modest otter
jfc
patent pivot
8 years here
modest otter
I do not belong here
patent pivot
yeah tokei is awesome, can't remember when I got it but I've used it forever
mellow hare
@modest otter We all started somewhere
mellow hare
I was
modest otter
oh
mellow hare
That was my first project I was actually happy with
gusty sonnet
Same actually, I picked up python and discord.py at the same time
molten bough
I didn't start with a discord bot, but you can basically ignore my path
it was totally the wrong way to go
haha
gusty sonnet
Luckily I was exposed to discord.js and the discord API so it wasn't that hard
modest otter
yeah. Barely have any experience with python, yet Im trying to create a bot by myself. Only asking for help from Python members
patent pivot
total commit counts
bot: 2579
branding: 125
seasonalbot: 1142
site: 1629
site.wiki: 44
snekbox: 321
Lurking the server definitely helped back then, so I have around 1.5 year of experience in python
molten bough
whew
that's a lot of commits
I expected site to have more than bot though for some reason
mellow hare
And there's absolutely nothing wrong with getting help
patent pivot
5,840 in total
I'd have expected that as well gdude with Django rewrite but apparently not ¯_(ツ)_/¯
cold moon
What should head command name? https://github.com/python-discord/seasonalbot/pull/364#pullrequestreview-369915678
green oriole
The space cog
clever wraith
If i was you , i would seperate the commands
hardy gorge
@clever wraith Separate them how?
hardy gorge
No, we want them as subcommands
There's no need to have that many top-class commands for features that are very related
green oriole
It would make the help command too noisy too
clever wraith
personal choices
hardy gorge
Yes, I also don't really see a reason for why they should have separate top-level commands. They're all quering nasa, they're part of the same kind of functionality, and part of the same cog.
They all do similar things.
green oriole
It will reduce duplicate code too
hardy gorge
It also does not really make sense to me from a user-perspective: .pictureoftheday or .potd doesn't communicate what kind of picture of the day or where it's coming from
Something like .nasa potd/.nasa pictureoftheday does
or .space <subcommand>
green oriole
Ah, I need to check something about the implementation of this command
It sometimes happen that the daily image is a video, if it is the picture I’m thinking about
And downloading and re-uploading a full video wouldn’t be very great for the network
Okay, it isn’t re-uploading, that’s fine
clever wraith
also its called .apod astronomy picture of the day
btw
hardy gorge
That doesn't really make a lot of difference. The point stands that the four commands are highly related and grouping them makes sense.
green oriole
By the way, any news on this issue? https://github.com/python-discord/bot/issues/769
Might be also worth updating https://github.com/python-discord/bot/issues/553 now that the bot migrated to 3.8
brazen charm
Wasnt there an older mailing list issue?
green oriole
Nope
I brought the idea up a few times, but never wrote an issue
And I couldn't find one on the repo
brazen charm
green oriole
F***
Thanks you github search for your incompetence
I swear, mailing list didn't showed it up
Time to edit I guess
hardy gorge
It's been discussed a lot and we had a conversation about it in the staff meeting
I'll some details from that conversation to the latest issue
I don't really mind which one we keep
eternal owl
Will we have this in the future?
https://discordapp.com/channels/267624335836053506/354619224620138496/607277850818641928
hardy gorge
Probably not. We don't have regular seasonalbot events anymore. I'll keep it in the back of my mind for the Seasonal Bot-based event we're plannig for October, but it something that takes time and coordiantion to organize.
The last time we tried it no one showed up until after it had officially finished.
crude gyro
they weren't very successful. we didn't see more than maybe one or two people drop by during the hours we made ourselves available.
I consider it a failed event.
we'll still do ad-hoc voip stuff, though.
hardy gorge
I think it would be cool to do some live pair programming during the event we're planning for October
It'll probably fit the theme
Still a long time to go, though
crude gyro
for those of you who are using PyCharm for projects with lots of unit tests, here's a cool plugin:
https://plugins.jetbrains.com/plugin/10874-git-bisect-run
Say you have a failing unit test. You run the test in PyCharm and then select that test and open the bisect window. In here, you select a revision (a commit on your branch) where you know that unit test was working, and you select a revision where the unit test is failing.
now pycharm will start bisecting.
basically it will use a logarithmic selection strategy to run that unit test on commits in the history between the good and the bad revision until it is able to narrow it down and find out exactly which commit introduced the bug.
and it does this without any intervention from you
molten bough
That's pretty badass
crude gyro
you just go get a cup of coffee, and when you return, pycharm says "okay, this is the commit that fucked up your unit test"
and since it's logarithmic, it's very fast, even if there are thousands of commits to look through
this is of course possible through the cli as well with git bisect
and if you don't have a unit test, it's possible to test manually as well
for example, say you have a bot command that's broken. you do git bisect start . Next, you check out a commit where the problem is happening and do git bisect bad . Then you get a commit where the problem does not occur and do git bisect good . Now git starts selecting commits and checking them out for you, and you test your command, do git bisect good if the function works and bad if it fails
after a while, git will tell you which commit is responsible. takes a bit longer, but still beats trying to figure it out by just digging through history.
molten bough
Git bisect, also known as "who broke my fucking tests?" - super handy, amused I never thought to look for a plugin
Wonder why the plugin doesn't have that description on it, maybe more people would use it
