FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Implement eq, ne for slice by HyeockJinKim · Pull Request #107 · go-python/gpython · GitHub

/ gpython Public

Implement eq, ne for slice #107

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.

Sign up for GitHub

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

Merged
ncw merged 2 commits into from
Oct 8, 2019
Conversation 9 Commits 2 Checks 0 Files changed
Merged

Implement eq, ne for slice #107

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4e50a59
Implement eq, ne for slice
HyeockJinKim Sep 29, 2019
80d41f0
Add tests for slice
HyeockJinKim Sep 29, 2019
File filter

Filter by extension

Filter by extension .go  (2) .py  (1) All 2 file types selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
10 changes: 10 additions & 0 deletions py/bool.go
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,16 @@ func (a Bool) M__ne__(other Object) (Object, error) {
return True, nil
}

func notEq(eq Object, err error) (Object, error) {
if err != nil {
return nil, err
}
if eq == NotImplemented {
return eq, nil
}
return Not(eq)
}

// Check interface is satisfied
var _ I__bool__ = Bool(false)
var _ I__index__ = Bool(false)
Expand Down
25 changes: 25 additions & 0 deletions py/slice.go
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,31 @@ func (r *Slice) GetIndices(length int) (start, stop, step, slicelength int, err
return
}

func (a *Slice) M__eq__(other Object) (Object, error) {
b, ok := other.(*Slice)
if !ok {
return NotImplemented, nil
}

if a.Start != b.Start {
return False, nil
}

if a.Stop != b.Stop {
return False, nil
}

if a.Step != b.Step {
return False, nil
}

return True, nil
}

func (a *Slice) M__ne__(other Object) (Object, error) {

sbinet Sep 30, 2019

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

just thinking out loud, but any reason we don't implement one in terms of the other?

HyeockJinKim Sep 30, 2019

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I looked at the range.go and list.go implementations, so I implemented __eq__ and __ne__ in the same way.
Should I create and call a private function for the common part?

sbinet Sep 30, 2019

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

not necessarily. it was just a silly question from my end.
I was just wondering whether there were a more philosophical underlying reason for this.

consistency w/ other parts of the code SGTM.
(one could also argue for reducing the number of function calls)

HyeockJinKim Sep 30, 2019

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Looking at the dict.go and set.go, there is some code written that calls __eq__ in __ne__, so I called eq.

ncw Oct 1, 2019

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

looking at the dict.go code and set.go code it is actually wrong and I think your PR #110 fixes up the set case.

What you've written here is much better.

However the fact that we've made the same mistake more than once in the code base makes me think we should factor this out and in the process tighten it up more.

So how about something like this in slice, set, dict and anywhere else we make a __ne__ from an __eq__

func (a *Set) M__ne__(other Object) (Object, error) {
	return NotEq(a.M__eq__(other))
}

And then put something like this in py/bool.go

// NotEq is used to invert the result of an __eq__ call
func NotEq(eq Object, err error) (Object, error) {
	if err != nil {
		return nil, err
	}
	switch eq {
	case True:
		return False, nil
	case False:
		return True, nil
	case NotImplemented:
		return eq, nil
	}
	return nil, ExceptionNewf(TypeError, "__eq__ returned a non bool: (type %s)", eq.Type().Name)
}

(all code untested!)

HyeockJinKim Oct 1, 2019

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Should I handle values ​​other than True, False and NotImplemented?
Do we have to return an exception even if we handle it?

ncw Oct 1, 2019

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

As far as I know eq should only ever return True, False or NotImplemented but python can use any value as a truthy value so...

Perhaps more Pythonic might be this which is equivalent to using the not operator on the result.

func NotEq(eq Object, err error) (Object, error) {
	if err != nil {
		return nil, err
	}
	if eq == NotImplemented {
		return eq, nil
	}
	return Not(eq)
}

Do we have to return an exception even if we handle it?

We need to return the exception from the __eq__ call.

I just wrote NotEq like that do it had a convenient way to call it NotEq(a.M__eq__(other))

HyeockJinKim Oct 2, 2019

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I understand. Would you please review again?
And when this PR is merged, I will modify #110 to use NotEq.
return notEq(a.M__eq__(other))
}

func init() {
SliceType.Dict["start"] = &Property{
Fget: func(self Object) (Object, error) {
Expand Down
11 changes: 11 additions & 0 deletions py/tests/slice.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,15 @@
assert a.stop == 10
assert a.step == 1

assert slice(1).__eq__(slice(1))
assert slice(1) != slice(2)
assert slice(1) == slice(None, 1, None)
assert slice(0, 0, 0) == slice(0, 0, 0)

assert slice(0, 0, 1) != slice(0, 0, 0)
assert slice(0, 1, 0) != slice(0, 0, 0)
assert slice(1, 0, 0) != slice(0, 0, 0)
assert slice(0).__ne__(slice(1))
assert slice(0, None, 3).__ne__(slice(0, 0, 3))

doc="finished"
Back | FazBrowse Home | New Git URL