New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Pathlib weherever possible 2665 #2688

Merged

Bibo-Joshi merged 38 commits into python-telegram-bot:v14 from eldbud:pathlib-weherever-possible-2665

Oct 5, 2021

Contributor

eldbud commented Oct 2, 2021

Refactored usage of str objects and/or os module can be replaced with pathlib.Path across the project.
Hopefully managed to find all places in the project that can be updated.

Bibo-Joshi and others added 23 commits

September 15, 2021 17:09


          Temporarily enable tests for the v14 branch

331c4f5


          Move and Rename TelegramDecryptionError to telegram.error.PassportDec…

9dc30cf

…ryptionError (python-telegram-bot#2621)

* move telegramdecryptionerror to error.py

* Change error class name


          Add Code Comment Guidelines to Contribution Guide (python-telegram-bo…

b089cc9

…t#2612)

* feat: add docs about docs

* fix: improve looks

* fix: make link work

* fix: this looks better

* Improved markdown, updated link

* Less justifying

Co-authored-by: Bibo-Joshi <22366557+Bibo-Joshi@users.noreply.github.com>


          Improve Type Hinting for CallbackContext (python-telegram-bot#2587)

3a19383

* Fix incomplete type annotations for CallbackContext

Co-authored-by: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com>


          Add Custom pytest Marker to Ease Development (python-telegram-bot#2628)

414a18c

* Feat: Custom pytest marker

Co-authored-by: Bibo-Joshi <22366557+Bibo-Joshi@users.noreply.github.com>


          Make BasePersistence Methods Abstract (python-telegram-bot#2624)

8edb49d

* Make basepersistence methods abstractmethod

Signed-off-by: starry69 <starry369126@outlook.com>

Co-authored-by: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com>


          Refactor Initialization of Persistence Classes (python-telegram-bot#2604

cce9c18


          Remove __dict__ from __slots__ and drop Python 3.6 (python-telegr…

dc0207a

…am-bot#2619, python-telegram-bot#2636)


          Remove day_is_strict argument of JobQueue.run_monthly (python-telegra…

4af252f

…m-bot#2634)

Co-authored-by: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com>


          Drop Non-CallbackContext API (python-telegram-bot#2617)

b6e4b54


          Fix Signatures and Improve test_official (python-telegram-bot#2643)


          Remove Deprecated Functionality (python-telegram-bot#2644)

dbdd35e


          Add User Friendly Type Check For Init Of {Inline, Reply}KeyboardMarkup (

a7c7c82

python-telegram-bot#2657)


          Refine Dispatcher.dispatch_error (python-telegram-bot#2660)

8ac65fc

Co-authored-by: Harshil <37377066+harshil21@users.noreply.github.com>


          Warnings Overhaul (python-telegram-bot#2662)

66f0604


          Clear Up Import Policy (python-telegram-bot#2671)

4f21c06

Co-authored-by: Harshil <37377066+harshil21@users.noreply.github.com>


          Make InlineQuery.answer Raise ValueError (python-telegram-bot#2675)

5542a46


          merged from v14

1027e49


          looks done

ec0665f


          added self to AUTHORS.rst

f98883a


          Doc Fixes (python-telegram-bot#2597)

2cf9311

Co-authored-by: Harshil <37377066+harshil21@users.noreply.github.com>
Co-authored-by: poolitzer <25934244+Poolitzer@users.noreply.github.com>


          Merge branch 'v14' into pathlib-weherever-possible-2665

bf0919c


          final better utilisations of the Path module and some docstring update

eac94bd

Poolitzer linked an issue

that may be closed by this pull request

Use pathlib weherever possible #2665

Closed

Poolitzer requested changes

View reviewed changes

Member

Poolitzer left a comment

You have one failing test here and it actually seems related to your changes. You wanna give it a shot at fixing? I can't recreate it myself sadly, using windows + py3.7, maybe someone else has a plan.

telegram/ext/picklepersistence.py Outdated Show resolved Hide resolved

telegram/ext/picklepersistence.py Outdated Show resolved Hide resolved

Bibo-Joshi requested changes

View reviewed changes

Member

Bibo-Joshi left a comment

Thanks for your PR! I left a number of comments an questions :)
Apart from those:

Please double check that you added versioning directives everywhere where needed.
Just to clarify: Is there now any parameter that accepts Path objects in addition to what it accepted before? if so, please make sure that the tests cover both cases

telegram/ext/picklepersistence.py Outdated Show resolved Hide resolved

telegram/files/file.py Outdated Show resolved Hide resolved

telegram/files/file.py Outdated Show resolved Hide resolved

telegram/files/file.py Show resolved Hide resolved

telegram/files/file.py Outdated Show resolved Hide resolved

telegram/files/inputfile.py Outdated Show resolved Hide resolved

telegram/request.py Outdated Show resolved Hide resolved

tests/test_file.py Outdated Show resolved Hide resolved

tests/test_inputfile.py Outdated Show resolved Hide resolved

eldbud added 2 commits

October 3, 2021 02:29


          attended to review notes

38c6641


          fixed calls of PicklePersistence.__init__() with keyworded arg

d5fe778

Bibo-Joshi reviewed

View reviewed changes

telegram/ext/picklepersistence.py Outdated Show resolved Hide resolved

telegram/ext/picklepersistence.py Outdated Show resolved Hide resolved

telegram/ext/picklepersistence.py Outdated Show resolved Hide resolved

telegram/files/file.py Outdated Show resolved Hide resolved

telegram/files/file.py Outdated Show resolved Hide resolved

telegram/files/file.py Outdated Show resolved Hide resolved

telegram/files/file.py Outdated Show resolved Hide resolved

telegram/request.py Outdated Show resolved Hide resolved

telegram/ext/picklepersistence.py Outdated Show resolved Hide resolved

Bibo-Joshi reviewed

View reviewed changes

telegram/files/file.py Outdated Show resolved Hide resolved

harshil21 reviewed

View reviewed changes

Member

harshil21 left a comment

round 2 of reviews

telegram/dice.py Outdated Show resolved Hide resolved

telegram/ext/dispatcher.py Outdated Show resolved Hide resolved

telegram/ext/picklepersistence.py Outdated Show resolved Hide resolved

telegram/ext/picklepersistence.py Outdated Show resolved Hide resolved

telegram/files/file.py Outdated Show resolved Hide resolved

Bibo-Joshi requested changes

View reviewed changes

Member

Bibo-Joshi left a comment

Thanks for the new updates and sorry for making your merge-life harder unintentionally 😬
I think we're pretty much down to nitpicking details :)

telegram/ext/picklepersistence.py Outdated Show resolved Hide resolved

tests/test_persistence.py Show resolved Hide resolved

tests/test_request.py Outdated Show resolved Hide resolved

telegram/ext/dispatcher.py Outdated Show resolved Hide resolved

telegram/ext/picklepersistence.py Outdated Show resolved Hide resolved


          added parametrization for tests that deal with custom_path in test_fi…

654ea00

…le.py, added a parametrized test_filepath_argument_types in test_persistence.py, and added test_download in test_request.py

github-actions bot reviewed

View reviewed changes

github-actions bot left a comment

Hey there. Relax, I am just a little warning for the maintainers to release directly after merging your PR, otherwise we have broken examples and people might get confused :)


          bringing codecov back up

db9b6b1

github-actions bot reviewed

View reviewed changes

github-actions bot left a comment

Hey there. Relax, I am just a little warning for the maintainers to release directly after merging your PR, otherwise we have broken examples and people might get confused :)

Bibo-Joshi reviewed

View reviewed changes

telegram/ext/picklepersistence.py Outdated Show resolved Hide resolved

telegram/ext/picklepersistence.py Outdated Show resolved Hide resolved

tests/test_persistence.py Outdated Show resolved Hide resolved


          some final details

70403ed

github-actions bot reviewed

View reviewed changes

github-actions bot left a comment

Hey there. Relax, I am just a little warning for the maintainers to release directly after merging your PR, otherwise we have broken examples and people might get confused :)

Bibo-Joshi approved these changes

View reviewed changes

Member

Bibo-Joshi left a comment

If tests run and @harshil21 approves as well, we can merge :)


          attended to review notes

fa42e22

Contributor Author

eldbud commented Oct 4, 2021 •

edited

Loading

@Bibo-Joshi can you please role back your push to this branch? i can't push my changes, and some of yours i think break the test.

edit: reverted.

eldbud added 2 commits

October 5, 2021 00:11


          Merge branch 'pathlib-weherever-possible-2665' of https://github.com/…

ecc8248

…eldbud/python-telegram-bot into pathlib-weherever-possible-2665


          brought back filepath.with_name(filepath.name etc)

96af7a6

github-actions bot reviewed

View reviewed changes

github-actions bot left a comment

Hey there. Relax, I am just a little warning for the maintainers to release directly after merging your PR, otherwise we have broken examples and people might get confused :)


          Revert "brought back filepath.with_name(filepath.name etc)"

5e10bd1

This reverts commit 96af7a6.

github-actions bot reviewed

View reviewed changes

github-actions bot left a comment

Hey there. Relax, I am just a little warning for the maintainers to release directly after merging your PR, otherwise we have broken examples and people might get confused :)

Member

Bibo-Joshi commented Oct 5, 2021

The failures where unrelated. I've reverted the reversion 😉

Contributor Author

eldbud commented Oct 5, 2021

The failures where unrelated. I've reverted the reversion 😉

did you make sure the test passes? this is how i initially wrote it and it failed.
when updating user data, the data is saved in FILENAME_user_data. so _user_data needs to be appended to the end of the original filename. without it, the is_file assetion fails.

Does this not happen to you?

Member

Bibo-Joshi commented Oct 5, 2021

@eldbud I removed single_file=False, so we don't need the _user_data extension 😉 The one currently failing test is unrelated

eldbud requested review from harshil21 and Poolitzer

October 5, 2021 07:53

harshil21 approved these changes

View reviewed changes

Member

harshil21 left a comment

good now!

Bibo-Joshi merged commit 6965ae9 into python-telegram-bot:v14

Member

Bibo-Joshi commented Oct 5, 2021

Thank you for this contribution @eldbud ! Nice work :)

eldbud deleted the pathlib-weherever-possible-2665 branch

October 5, 2021 18:25

github-actions bot locked and limited conversation to collaborators

harshil21 added the hacktoberfest-accepted label

Bibo-Joshi added 🔌 enhancement and removed enhancement labels

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

🔌 enhancement hacktoberfest-accepted