dymik739
/
modular-bot-framework-for-telegram
1
0
Fork 2
Code Issues Pull Requests Projects Releases Wiki Activity

auto-schedule-pro-v2 #8

Manually merged
dymik739 merged 0 commits from Rhinemann/modular-bot-framework-for-telegram:auto-schedule-pro-v2 into auto-schedule-pro-v2 2023-06-27 16:11:40 +03:00
Conversation 2 Commits - Files Changed -
Rhinemann commented 2023-05-08 10:02:42 +03:00
Contributor
Copy Link

Formatted according to PEP8 and added f-strings, no initial issues found.

Formatted according to PEP8 and added f-strings, no initial issues found.
Rhinemann added 2 commits 2023-05-08 10:02:42 +03:00
Rhinemann commented 2023-05-08 10:03:38 +03:00
Author
Contributor
Copy Link

Oh, also fixing some typos too.

Oh, also fixing some typos too.
Rhinemann added 1 commit 2023-05-08 10:13:03 +03:00
Rhinemann added 1 commit 2023-05-08 10:20:54 +03:00
Rhinemann added 1 commit 2023-05-08 10:21:37 +03:00
Rhinemann added 1 commit 2023-05-08 10:26:57 +03:00
dymik739 requested changes 2023-05-08 12:13:59 +03:00
dymik739 left a comment
Owner
Copy Link

Overall, it's a great pull request;

There is one thing that is missing though: all the changes to module-testing.py are not mirrored to main.py for the deployment (we probably need a script or something that will automatically copy all the changes from module-testing.py to main.py and vice-versa)

Also, there are some details I've described in the code comments, but they're all minor) (and "понеділок"...)

Overall, it's a great pull request; There is one thing that is missing though: all the changes to module-testing.py are not mirrored to main.py for the deployment (we probably need a script or something that will automatically copy all the changes from module-testing.py to main.py and vice-versa) Also, there are some details I've described in the code comments, but they're all minor) (and "понеділок"...)
module-testing.py
@ -16,3 +17,3 @@
def readfile(filename):
try:
return codecs.open(filename, encoding = "utf-8").read()
return codecs.open(filename, encoding="utf-8").read()
dymik739 commented 2023-05-08 11:41:12 +03:00
Owner
Copy Link

The standart states that there should be no spaces between key=value arguments;
I don't know why, but I've always written those with spaces just so that the code feels more spacious and free (basically, it's just a personal preference, which is wrong in terms of PEP8 standart; I'll probably get rid of it at some point (just like with using f-strings instead of str.format() method), but it might take some time)

The standart states that there should be no spaces between key=value arguments; I don't know why, but I've always written those with spaces just so that the code feels more spacious and free (basically, it's just a personal preference, which is wrong in terms of PEP8 standart; I'll probably get rid of it at some point (just like with using f-strings instead of `str.format()` method), but it might take some time)
Rhinemann commented 2023-05-08 12:18:44 +03:00
Author
Contributor
Copy Link

Okay, sure thing.

Okay, sure thing.
module-testing.py
@ -154,3 +160,3 @@
# message queue object to go back to synchronous message processing
#class MessageQueue:
# class MessageQueue:
dymik739 commented 2023-05-08 11:48:16 +03:00
Owner
Copy Link

That commented block is not really used for anything, so we can probably just delete it)

That commented block is not really used for anything, so we can probably just delete it)
Rhinemann commented 2023-05-08 12:20:07 +03:00
Author
Contributor
Copy Link

Agreed.

Agreed.
module-testing.py
@ -225,3 +231,3 @@
mcu = ModuleControlUnit()
processor_thread = threading.Thread( target = queue_processor, args = [] )
processor_thread = threading.Thread(target=queue_processor, args=[])
dymik739 commented 2023-05-08 12:02:00 +03:00
Owner
Copy Link

The same as above, but I'm fine with removing spaces around all arguments like so:
Thread( target = queue_processor, args = [] ) -> Thread(target = queue_processor, args = [])
(hopefully I'll learn to put spaces correctly at some point)

The same as above, but I'm fine with removing spaces around all arguments like so: `Thread( target = queue_processor, args = [] ) -> Thread(target = queue_processor, args = [])` (hopefully I'll learn to put spaces correctly at some point)
modules/auto-schedule-pro-v2/main.py
@ -9,3 +11,2 @@
# Accusative - znahidnyj
WEEKDAYS_ACCUSATIVE = ["понедулок", "вівторок", "середу", "четвер",
"п'ятницю", "суботу", "неділю"]
WEEKDAYS_ACCUSATIVE = ["понеділок", "вівторок", "середу", "четвер", "п'ятницю", "суботу", "неділю"]
dymik739 commented 2023-05-08 12:09:26 +03:00
Owner
Copy Link

понеділок)

понеділок)
modules/auto-schedule-pro-v2/main.py
@ -29,2 +26,3 @@
human_readable_date = ""
if ((current_day + 2) == int(start_datetime.strftime("%u"))) or ((current_day == 6) and (start_datetime.strftime("%u") == "1")):
if ((current_day + 2) == int(start_datetime.strftime("%u"))) or (
(current_day == 6) and (start_datetime.strftime("%u") == "1")):
dymik739 commented 2023-05-08 12:05:08 +03:00
Owner
Copy Link

Sometimes I question what do people developing PEP8 consider "readable"...
This change is fine, even though it scares me

Sometimes I question what do people developing PEP8 consider "readable"... This change is fine, even though it scares me
Rhinemann commented 2023-05-08 12:25:04 +03:00
Author
Contributor
Copy Link

That one is weird, we can tweak it a bit.
Like this, for example:

if ((current_day + 2) == int(start_datetime.strftime("%u"))) or \
            ((current_day == 6) and (start_datetime.strftime("%u") == "1")):

Seems a tad better to me.

That one is weird, we can tweak it a bit. Like this, for example: ```python if ((current_day + 2) == int(start_datetime.strftime("%u"))) or \ ((current_day == 6) and (start_datetime.strftime("%u") == "1")): ``` Seems a tad better to me.
❤️ 1
dymik739 commented 2023-05-08 19:52:20 +03:00
Owner
Copy Link

Yeah, this looks way better) (it's a very minor detail, but, for some reason, stray parenthese on a line bothers me whenever I look at it whereas a simple backslash looks absolutely fine)

Yeah, this looks way better) (it's a very minor detail, but, for some reason, stray parenthese on a line bothers me whenever I look at it whereas a simple backslash looks absolutely fine)
Rhinemann added 1 commit 2023-05-08 12:26:12 +03:00
Rhinemann added 1 commit 2023-05-08 12:43:33 +03:00
Rhinemann reviewed 2023-05-08 12:50:18 +03:00
main.py
@ -71,2 +75,2 @@
responce = self.obj.process(msg, self.path)
return responce
response = self.obj.process(msg, self.path)
return response
Rhinemann commented 2023-05-08 12:49:55 +03:00
Author
Contributor
Copy Link

By the way, maybe we can try

            return self.obj.process(msg, self.path)

As an option? Since the variable isn't really used.

By the way, maybe we can try ```python return self.obj.process(msg, self.path) ``` As an option? Since the variable isn't really used.
dymik739 commented 2023-05-08 19:44:05 +03:00
Owner
Copy Link

That's a very good call) (I'm not sure why have I created that variable in the first place, but yes, it seems to not have a purpose)

We may address this after the PR merge, because it's relevant to the master branch and not to the auto-schedule-pro-v2 (this branch will probably be merged into master and deleted once this PR is complete and merged)

That's a very good call) (I'm not sure why have I created that variable in the first place, but yes, it seems to not have a purpose) We may address this after the PR merge, because it's relevant to the master branch and not to the auto-schedule-pro-v2 (this branch will probably be merged into master and deleted once this PR is complete and merged)
dymik739 commented 2023-05-08 20:20:45 +03:00
Owner
Copy Link

Also, there is one occurance of responce_text not changed in file modules/qna-basic/db/obj.json which breaks the module (sure, it hasn't been touched for about 9 months now, but it might become useful at some point, so it's worth keeping)

Also, there is one occurance of `responce_text` not changed in file `modules/qna-basic/db/obj.json` which breaks the module (sure, it hasn't been touched for about 9 months now, but it might become useful at some point, so it's worth keeping)
dymik739 requested changes 2023-05-08 20:33:16 +03:00
dymik739 left a comment
Owner
Copy Link

Everything is alright besides modules/qna-basic/ being broken by the API change (it failed to load response from the database during the local testing)

For some reason, I can't fork your fork to make a PR regarding this PR (I wanted to push a commit addressing this into this PR; maybe, there is another way to push a commit into a PR?), but the fix is really simple; in the file modules/qna-basic/db/obj.json the dictionary key should be changed at line 5 like this: responce_text -> response_text

Everything is alright besides `modules/qna-basic/` being broken by the API change (it failed to load response from the database during the local testing) For some reason, I can't fork your fork to make a PR regarding this PR (I wanted to push a commit addressing this into this PR; maybe, there is another way to push a commit into a PR?), but the fix is really simple; in the file `modules/qna-basic/db/obj.json` the dictionary key should be changed at line 5 like this: `responce_text -> response_text`
dymik739 manually merged commit 44628a3021 into auto-schedule-pro-v2 2023-06-27 16:11:40 +03:00
Sign in to join this conversation.
Reviewers
No reviewers
dymik739
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: dymik739/modular-bot-framework-for-telegram#8
No description provided.
Delete Branch "Rhinemann/modular-bot-framework-for-telegram:auto-schedule-pro-v2"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?