python-twitter-tools / twitter

Python Twitter API
http://mike.verdone.ca/twitter/
MIT License
3.17k stars 710 forks source link

Using a ssl context object to avoid deprecation warnings in py3 #451

Closed Yomguithereal closed 1 year ago

hugovk commented 1 year ago

Please could you add comments to say which bit is for Python 2 and which for 3?

That will make it easier to clean up the code when the time comes.

coveralls commented 1 year ago

Pull Request Test Coverage Report for Build 3052768982

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details


Changes Missing Coverage Covered Lines Changed/Added Lines %
twitter/api.py 5 6 83.33%
twitter/stream.py 0 6 0.0%
<!-- Total: 5 12 41.67% -->
Totals Coverage Status
Change from base Build 3051803496: 0.05%
Covered Lines: 655
Relevant Lines: 2088

💛 - Coveralls
Yomguithereal commented 1 year ago

Please could you add comments to say which bit is for Python 2 and which for 3?

The code I wrote works the same in py2 and py3 as far as I know so nothing to clean up when the time comes (to kill py2 I suppose?). It's just that the earlier PR was triggering deprecation warning on py >= 3.6. Now it does not.

boogheta commented 1 year ago

I'm the one who sollicitated the PR to @Yomguithereal following the previous one. This solves an issue we met while using the lib on recent Mac where python could not find on its own where the certifs resides on the OS. This makes me wonder actually whether we should add to the matrix some tests under macos/windows as well, although that's already a lot of tweets sent whenever tests are run so not sure. What do you think @hugovk?

hugovk commented 1 year ago

I think it's a good idea to test at least one on Windows and macOS. We don't necessarily need to add them to the full matrix to get the benefit.

We currently have:

            "2.7",
            "3.6",
            "3.7",
            "3.8",
            "3.9",
            "3.10",
            "3.11-dev",
            "pypy2.7",

We could switch out one of these middle versions to run on Windows instead of Ubuntu, and another to run on macOS.

In fact, we could remove some of those middle Ubuntu ones anyway, some projects only test on lower and upper bounds. So for example:

            "2.7",
            "3.6",
-           "3.7",
-           "3.8",
-           "3.9",
            "3.10",
            "3.11-dev",
            "pypy2.7",

Also Python 3.6 has been EOL since last year (2021-12-23) so that could be dropped. (Plus I still recommend EOL 2.7 being dropped (2020-01-01) ;)

boogheta commented 1 year ago

I like the idea to test some versions with mac and windows, for instance we could remove 3.7, and run 3.8 under windows and 3.9 under macOS ? I know for 2.7, but I kinda like that the code is robust enough that it still supports it so far and I would be in favor of keeping it unless it becomes a maintenance charge, but that's only my opinion :)

hugovk commented 1 year ago

Sounds good, I'll create a PR. Fine with me if you'd like to keep 2.7 longer. How about 3.6?

boogheta commented 1 year ago

Ha I misread you, then let's rather say drop 3.6, keep 3.7 on ubuntu, 3.8 windows and 3.9 mac?

hugovk commented 1 year ago

Please see PR https://github.com/python-twitter-tools/twitter/pull/453