Skip to content

Fix #291: drop python < 3.6#304

Merged
mergify[bot] merged 1 commit intojd:masterfrom
penguinolog:py36+_only
Jun 23, 2021
Merged

Fix #291: drop python < 3.6#304
mergify[bot] merged 1 commit intojd:masterfrom
penguinolog:py36+_only

Conversation

@penguinolog
Copy link
Copy Markdown
Contributor

  • Removed all transitional requirements (including six)
  • Most type annotations fixed.
  • Python 3.10 tested.
  • Most part of code is type annotated.

Comment thread tenacity/__init__.py
Comment thread tenacity/__init__.py
Comment thread tenacity/__init__.py
Comment thread tenacity/__init__.py
Comment thread tenacity/__init__.py
Comment thread tenacity/__init__.py


class BaseRetrying(object):
__metaclass__ = ABCMeta
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

looks like here was s bug

Comment thread tenacity/__init__.py
return fut.result()

if self.after is not None:
self.after(retry_state=retry_state)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

do not use keyword call

Comment thread tenacity/_utils.py
from monotonic import monotonic as now # noqa


class cached_property(object):
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

not used at code base -> no reason to migrate on backports or make semi-stdlib code

def test_repr(self):
class ConcreteRetrying(tenacity.BaseRetrying):
def __call__(self):
def __call__(self, fn, *args, **kwargs):
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

do not modify signature

Comment thread tenacity/__init__.py
)
if iscoroutinefunction is not None and iscoroutinefunction(f):
r = AsyncRetrying(*dargs, **dkw)
r: "BaseRetrying" = AsyncRetrying(*dargs, **dkw)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

using base class due to API reuse and MyPy questions

Comment thread tenacity/__init__.py
break

@abstractmethod
def __call__(self, *args, **kwargs):
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

API should match between parent and child classes

Comment thread tenacity/__init__.py
def call(self, *args, **kwargs):
def call(self, *args: t.Any, **kwargs: t.Any) -> t.Union[DoAttempt, DoSleep, t.Any]:
"""Use ``__call__`` instead because this method is deprecated."""
warnings.warn(
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

looks like reason to set deprecation period before remove (and also for bound function in Retrying)

Comment thread tenacity/_utils.py


def visible_attrs(obj, attrs=None):
def visible_attrs(
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

method is used only once and technically 1liner -> dict comprehension in place of use?

Comment thread tenacity/compat.py Outdated
jd
jd previously requested changes Jun 23, 2021
Copy link
Copy Markdown
Owner

@jd jd left a comment

Choose a reason for hiding this comment

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

A couple of minor things otherwise LGTM.
(I'd have preferred to have multiple PR for different things here: removing Python 2 is one thing, adding typing information is another.)

Thanks for doing this huge work!

Comment thread releasenotes/notes/py36_plus-c425fb3aa17c6682.yaml Outdated
Comment thread releasenotes/notes/py36_plus-c425fb3aa17c6682.yaml Outdated
Comment thread releasenotes/notes/py36_plus-c425fb3aa17c6682.yaml Outdated
Comment thread releasenotes/notes/py36_plus-c425fb3aa17c6682.yaml Outdated
Comment thread tenacity/__init__.py
@penguinolog
Copy link
Copy Markdown
Contributor Author

(I'd have preferred to have multiple PR for different things here: removing Python 2 is one thing, adding typing information is another.)

#293 is repared, so I can rebase on it after merge. It will reduce size of mine PR

@mergify mergify Bot dismissed jd’s stale review June 23, 2021 08:10

Pull request has been modified.

@penguinolog penguinolog marked this pull request as draft June 23, 2021 08:19
@penguinolog penguinolog marked this pull request as ready for review June 23, 2021 09:42
@penguinolog
Copy link
Copy Markdown
Contributor Author

Rebased on master

@penguinolog penguinolog requested a review from jd June 23, 2021 12:05
jd
jd previously requested changes Jun 23, 2021
Copy link
Copy Markdown
Owner

@jd jd left a comment

Choose a reason for hiding this comment

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

Ditto for the six and typing which should be 2 different PR, but if it's complicated to split them, it'll be fine.

Comment thread releasenotes/notes/py36_plus-c425fb3aa17c6682.yaml Outdated
Comment thread tenacity/__init__.py
Comment thread tenacity/before.py


def before_log(logger, log_level):
def before_log(logger: "logging.Logger", log_level: int) -> typing.Callable[["RetryCallState"], None]: # noqa:BLK100
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

just run black?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Will make separate PR with unified black formatting

Comment thread tenacity/compat.py Outdated
@mergify mergify Bot dismissed jd’s stale review June 23, 2021 12:42

Pull request has been modified.

@penguinolog penguinolog requested a review from jd June 23, 2021 12:47
* Removed all transitional requirements (including `six`)
* Most type annotations fixed.
* Python 3.10 tested.
* Most part of code is type annotated.

Co-authored-by: Julien Danjou <julien@danjou.info>
@penguinolog
Copy link
Copy Markdown
Contributor Author

manually rebased to make history float

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.

2 participants