Skip to content

Use 'property' with 'abstractmethod' instead 'abstractproperty'#5456

Merged
thedrow merged 2 commits into
celery:v5-devfrom
tothegump:del-abstractproperty
Aug 5, 2020
Merged

Use 'property' with 'abstractmethod' instead 'abstractproperty'#5456
thedrow merged 2 commits into
celery:v5-devfrom
tothegump:del-abstractproperty

Conversation

@tothegump

Copy link
Copy Markdown
Contributor

'@abc.abstractproperty' has deprecated since Python 3.3
It is now possible to use property, property.getter(), property.setter() and property.deleter() with abstractmethod(), making this decorator redundant.
Python doc

@thedrow thedrow self-assigned this Apr 11, 2019
@thedrow thedrow added this to the 4.3.x Maintenance milestone Apr 11, 2019

@auvipy auvipy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

thats great!!

@thedrow

thedrow commented May 13, 2019

Copy link
Copy Markdown
Contributor

What about 2.7?

@auvipy auvipy removed this from the 4.4.0 milestone May 13, 2019
@tothegump

Copy link
Copy Markdown
Contributor Author

You are right, Python 2.7 can not use 'property' with 'abstractmethod'.
What should I do then? Which one is better choice:

  1. close this PR
  2. Waiting for Celery 5. (p.s. The branch 5.0-devel latest commit date is Feb 17, 2017, does it is in progress?)
  3. write extra compatible code for Python 2.7

@auvipy

auvipy commented May 17, 2019

Copy link
Copy Markdown
Member

You are right, Python 2.7 cannot use 'property' with 'abstractmethod'.
What should I do then? Which one is a better choice:

  1. close this PR
  2. Waiting for Celery 5. (p.s. The branch 5.0-devel latest commit date is Feb 17, 2017, does it is in progress?)
  3. write an extra compatible code for Python 2.7

wait for c5

@codecov

codecov Bot commented Oct 23, 2019

Copy link
Copy Markdown

Codecov Report

Merging #5456 into v5-dev will decrease coverage by 0.46%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           v5-dev    #5456      +/-   ##
==========================================
- Coverage   83.26%   82.79%   -0.47%     
==========================================
  Files         144      144              
  Lines       16523    16491      -32     
  Branches     2096     2057      -39     
==========================================
- Hits        13758    13654     -104     
- Misses       2549     2625      +76     
+ Partials      216      212       -4
Impacted Files Coverage Δ
celery/utils/abstract.py 21.05% <0%> (-3.95%) ⬇️
celery/loaders/__init__.py 14.28% <0%> (-5.72%) ⬇️
celery/concurrency/__init__.py 14.28% <0%> (-5.72%) ⬇️
celery/app/__init__.py 60.71% <0%> (-4.68%) ⬇️
celery/utils/objects.py 66.66% <0%> (-4.17%) ⬇️
celery/app/backends.py 59.37% <0%> (-3.96%) ⬇️
celery/app/annotations.py 60.6% <0%> (-3.92%) ⬇️
celery/app/registry.py 45.45% <0%> (-2.94%) ⬇️
celery/utils/iso8601.py 66.66% <0%> (-2.9%) ⬇️
... and 135 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 876b9df...5c0d093. Read the comment docs.

@thedrow thedrow added this to the v5.0.0 milestone Oct 23, 2019
@thedrow

thedrow commented Oct 23, 2019

Copy link
Copy Markdown
Contributor

I've rebased this to be merged on Celery 5.
This should be merged to that branch.

@auvipy

auvipy commented Oct 23, 2019

Copy link
Copy Markdown
Member

great

@auvipy

auvipy commented Oct 23, 2019

Copy link
Copy Markdown
Member

should we modify some tests to avoid any regression?

@thedrow

thedrow commented Oct 23, 2019

Copy link
Copy Markdown
Contributor

Which tests do you have in mind?

@thedrow
thedrow merged commit 1c11eb8 into celery:v5-dev Aug 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants