Skip to content
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.

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

Remove __ne__ methods #7257

Merged
merged 2 commits into from
Mar 6, 2022
Merged

Remove __ne__ methods #7257

merged 2 commits into from
Mar 6, 2022

Conversation

atombrella
Copy link
Contributor

I'm not really sure what the sentiments are for this type of change. Python's documentation was slightly unclear to me, and I wrote some sample code to clarify how it works.

https://gist.github.com/atombrella/c16693f564b4bbf375f70e5d56fe30ad

I think this __ne__ method can be removed as well:

def with_unique_field(attr):

        def __ne__(this, other):
            res = this.__eq__(other)
            return True if res is NotImplemented else not res
        cls.__ne__ = __ne__

Description

These are already defined as the opposite of eq in Python 3, and when eq
returns NotImplemented, Python by default will return True.

Copy link
Member

@auvipy auvipy left a comment

Choose a reason for hiding this comment

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

please check the test failures as well

@lgtm-com
Copy link

lgtm-com bot commented Jan 24, 2022

This pull request introduces 1 alert and fixes 2 when merging 1d0dee3 into fdb4af3 - view on LGTM.com

new alerts:

  • 1 for Unused import

fixed alerts:

  • 1 for Non-exception in 'except' clause
  • 1 for Module is imported with 'import' and 'import from'

@atombrella
Copy link
Contributor Author

@auvipy I'm not quite sure my changes caused the test failures. They all appear to be related to codecov connectivity/token issues. If you can re-trigger the jobs, and they still fail, I'll have a closer look. The tox steps all pass, but codecov does not.

@codecov
Copy link

codecov bot commented Jan 24, 2022

Codecov Report

Merging #7257 (30e13ce) into master (25ca389) will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7257      +/-   ##
==========================================
+ Coverage   89.31%   89.32%   +0.01%     
==========================================
  Files         138      138              
  Lines       16774    16745      -29     
  Branches     2450     2448       -2     
==========================================
- Hits        14982    14958      -24     
+ Misses       1560     1555       -5     
  Partials      232      232              
Flag Coverage Δ
unittests 89.32% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
celery/beat.py 91.50% <ø> (-0.05%) ⬇️
celery/events/state.py 97.22% <ø> (-0.03%) ⬇️
celery/result.py 96.28% <ø> (-0.07%) ⬇️
celery/schedules.py 84.19% <ø> (+0.65%) ⬆️
celery/utils/collections.py 85.84% <ø> (+0.15%) ⬆️
celery/backends/asynchronous.py 68.83% <0.00%> (ø)

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 25ca389...30e13ce. Read the comment docs.

@auvipy auvipy added this to the 5.2.x milestone Jan 24, 2022
@lgtm-com
Copy link

lgtm-com bot commented Jan 24, 2022

This pull request introduces 4 alerts and fixes 1 when merging f749ae9 into fdb4af3 - view on LGTM.com

new alerts:

  • 2 for Wrong name for an argument in a call
  • 1 for Unused import
  • 1 for Clear-text logging of sensitive information

fixed alerts:

  • 1 for Module is imported with 'import' and 'import from'

These are already defined as the opposite of __eq__ in Python 3, and when __eq__
returns NotImplemented, Python by default will return True.
@lgtm-com
Copy link

lgtm-com bot commented Jan 25, 2022

This pull request introduces 25 alerts and fixes 2 when merging 30e13ce into 25ca389 - view on LGTM.com

new alerts:

  • 12 for Wrong number of arguments in a call
  • 10 for Clear-text logging of sensitive information
  • 2 for Wrong name for an argument in a call
  • 1 for `__eq__` not overridden when adding attributes

fixed alerts:

  • 1 for Module is imported with 'import' and 'import from'
  • 1 for Wrong name for an argument in a class instantiation

@auvipy auvipy requested a review from a team February 2, 2022 13:35
@auvipy auvipy merged commit 4a6bdb2 into celery:master Mar 6, 2022
@atombrella atombrella deleted the ne_methods branch March 6, 2022 10:03
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.

None yet

2 participants