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

lib: Updated __cmp__ method for Python3 functionality #4684

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

arohanajit
Copy link
Contributor

wrt to #1809
This PR updates the __cmp__ method in saferef.py to be compatible with Python 3. Comparison methods __eq__
and __lt__ are implemented to maintain the same functionality without requiring changes elsewhere in the code.

Changes:

  • Added __eq__ method for equality comparison.
  • Added __lt__ method for less-than comparison.
  • The __cmp__ method now uses the rich comparison methods for its logic.

This shouldn't change overall behavior of saferef class.

@github-actions github-actions bot added Python Related code is in Python libraries labels Nov 11, 2024
@arohanajit arohanajit changed the title grass: Updated __cmp__ method for Python3 functionality lib: Updated __cmp__ method for Python3 functionality Nov 11, 2024
@petrasovaa
Copy link
Contributor

@echoix any suggestions here? Upstream has no fix for it. It's not used.

@echoix
Copy link
Member

echoix commented Nov 11, 2024

@echoix any suggestions here? Upstream has no fix for it. It's not used.

I'd need to have a deep look for this, as it might be impactful. I think we needed to revert one of my PRs that touched this if I remember well.

I'd be cautious and read well. Was it @wenzeslaus that has experience with this file?

Copy link
Contributor

@petrasovaa petrasovaa left a comment

Choose a reason for hiding this comment

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

I would remove the __cmp__ (it is not used in Python3) and keep the rest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libraries Python Related code is in Python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants