Skip to content

Conversation

Aweryc
Copy link
Contributor

@Aweryc Aweryc commented Jul 14, 2025

Convenience Functionality for BusinessOpeningHours
-check if the business is open at a given time
-get the opening hours for a given day

Aweryc and others added 3 commits July 14, 2025 13:00
-check if the business is open at a given time
-get the opening hours for a given day
Convenience Functionality for BusinessOpeningHours -check if the business is open at a given time -get the opening hours for a given day
@Bibo-Joshi Bibo-Joshi linked an issue Jul 15, 2025 that may be closed by this pull request
Copy link
Member

@Bibo-Joshi Bibo-Joshi left a comment

Choose a reason for hiding this comment

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

Hey. Thanks for your PR!

I gave it first quick look, but without actually reviewing the new logic. For starters, I focused on a few formalities and setup problems since you're apprantly strugging with this #4194 (comment). I hope that my comments help you.
Please also make sure to check the output of the pre-commit ci run in the "status checks" tab on the PR.

Please let me know if you need additional help with getting the tests running & pre-commit to pass - or when that's done and I should review properly :) Looking forward to it!

Aweryc and others added 12 commits July 15, 2025 23:56
Co-authored-by: Bibo-Joshi <22366557+Bibo-Joshi@users.noreply.github.com>
Co-authored-by: Bibo-Joshi <22366557+Bibo-Joshi@users.noreply.github.com>
Co-authored-by: Bibo-Joshi <22366557+Bibo-Joshi@users.noreply.github.com>
Co-authored-by: Bibo-Joshi <22366557+Bibo-Joshi@users.noreply.github.com>
edit 4861.HEoGVs2mYXWzqMahi6SEhV.toml
@Aweryc
Copy link
Contributor Author

Aweryc commented Jul 17, 2025

Hello. Im having general problems to understand work flow here in git PR )
Some (requested here) changes are applied successfully. Nice.
Some not. After every commit start kind a pack of git actions, right?
Yes I see trace back of errors in a git actions - but have no idea what it means.
I have strong feelings that is not because my files with my code fails it. May be I am wrong)

Please guide me)

@Poolitzer
Copy link
Member

Poolitzer commented Jul 17, 2025

Hi I am going to give this a shot.

Some (requested here) changes are applied successfully. Nice.
Some not. After every commit start kind a pack of git actions, right?

I do not quite get the question here, which requested changed did you not apply successfully? Yes the git actions will run on every commit. Doesn't really matter, the important part is that they don't fail on the last one.

Yes I see trace back of errors in a git actions - but have no idea what it means.

The pre commit CI is imo pretty self explanatory, you apparently in the second PR also already fixed some issues there, so I am going to assume you understand that. You can also change the branch in this PR to your new one in your repo.

We can ignore the Bot API test, and fix the chango/security analysis later. The important part to ensure you code is working correctly is the pre commit and the unit test.

If you open any unit test run, and wait for it to load, you should have a huge wall of text. Scroll up to leave the flaky tests behind, until you see tests/test_business_classes.py:715: AssertionError. These are now the unit tests which failed, and they explain you where it failed and why (what it expected, what it got). You need to scroll up further to see them all, but you will get quickly an idea what the output format is. So either you need to fix the tests, or your code. Do that for all the tests, and see where that gets you.

If you have more questions, make sure you read through the contrib guidelines and otherwise ask. Just here, or in our development group.

@Aweryc
Copy link
Contributor Author

Aweryc commented Jul 18, 2025

On my pc i have this

(python-telegram-bot) PS C:\Users\Aweryc\PycharmProjects\python-telegram-bot> pre-commit run -a
ruff.....................................................................Passed
black....................................................................Passed
flake8...................................................................Passed
pylint...................................................................Passed
mypy-ptb.................................................................Passed
mypy-examples............................................................Passed
pyupgrade................................................................Passed
isort....................................................................Passed

Now go to CI steps)

Aweryc and others added 8 commits July 18, 2025 16:19
# Conflicts:
#	changes/unreleased/4861.HEoGVs2mYXWzqMahi6SEhV.toml
#	src/telegram/_utils/datetime.py
# Conflicts:
#	changes/unreleased/4861.HEoGVs2mYXWzqMahi6SEhV.toml
#	src/telegram/_utils/datetime.py
@Aweryc
Copy link
Contributor Author

Aweryc commented Jul 21, 2025

Hello. I made a changes and now some unit tests are passed for different python versions, some ain't.
It is failing just in one test:

test_send_paid_media: tests._files.test_inputmedia.TestSendMediaGroupWithRequest

What should I do?

@Aweryc Aweryc requested a review from Bibo-Joshi July 21, 2025 11:13
Copy link
Member

@Bibo-Joshi Bibo-Joshi left a comment

Choose a reason for hiding this comment

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

Heyho :) Thanks for the updates and sorry for the late reply. I've gone through your code logic now and left several comments, but haven't reviewed the tests yet. As for the failing tests, I opened #4879. Once that's merged, we'll apply it to your branch as well :)

@Bibo-Joshi
Copy link
Member

PS: #4879 is merged. You can merge the master branch into yours :)

@Bibo-Joshi
Copy link
Member

@Aweryc are you still interseting in completing the work on this?

@Aweryc
Copy link
Contributor Author

Aweryc commented Aug 10, 2025

@Aweryc are you still interseting in completing the work on this?

Yes. Have had a hard times last two weeks. Next Tuesday i will start work on this again.

@Aweryc
Copy link
Contributor Author

Aweryc commented Aug 17, 2025

Hello) I made a changes but again do not understand why tests are failing )
this says

Not waiting for flood control: Flood control exceeded. Retry in 25205 seconds

Copy link
Member

@Bibo-Joshi Bibo-Joshi left a comment

Choose a reason for hiding this comment

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

Thanks very much for the updates!

Unfortunately I left quite a few comments again. I tried to be more specific to avoid new misunderstnadings. Please let me know if something is still unclear. I had a look at test_datetime this time due to the failing tests, but haven't gotten to test_business_classes yet …

Not waiting for flood control: Flood control exceeded. Retry in 25205 seconds

This is due to our test setup which uses real bots on Telegram - they get flood limits every now and then when a lot of tests are being run. You can ignore tests that failing due to this …

Aweryc and others added 3 commits August 23, 2025 16:29
Co-authored-by: Bibo-Joshi <22366557+Bibo-Joshi@users.noreply.github.com>
Co-authored-by: Bibo-Joshi <22366557+Bibo-Joshi@users.noreply.github.com>
Copy link
Member

@Bibo-Joshi Bibo-Joshi left a comment

Choose a reason for hiding this comment

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

sorry for the late reply

@Bibo-Joshi
Copy link
Member

I took the liberty to go ahead and implement my comments :)

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

Successfully merging this pull request may close these issues.

Convenience Functionalty for BusinessOpeningHours
3 participants