close
The Wayback Machine - https://web.archive.org/web/20200905104009/https://github.com/google/grumpy/pull/270
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

Implement floordiv and NotImplemented.__repr__() #270

Merged
merged 1 commit into from Mar 23, 2017

Conversation

@corona10
Copy link
Contributor

corona10 commented Mar 21, 2017

  1. Implement floordiv.
    This PR is related with #269
  2. Implement NotImplemented.repr()
@corona10 corona10 changed the title [WIP] Import ouroboros's operator and implement FloorDiv [WIP] Import modified ouroboros's operator and implement FloorDiv Mar 21, 2017
Copy link
Collaborator

trotterdylan left a comment

I know this is still a WIP, just thought I'd provide a few suggestions.

@@ -98,6 +98,22 @@ func intEq(f *Frame, v, w *Object) (*Object, *BaseException) {
return intCompare(compareOpEq, toIntUnsafe(v), w), nil
}

func intFloorDiv(f *Frame, v, w *Object) (*Object, *BaseException) {
if !v.isInstance(IntType) {
return nil, f.RaiseType(TypeErrorType, fmt.Sprintf("descriptor '__floordiv__' requires a 'int' object but received a '%s'", v.typ.Name()))

This comment has been minimized.

@trotterdylan

trotterdylan Mar 22, 2017

Collaborator

The slot delegation code guarantees that the first argument to slot functions is of the correct type, so no need to check here.

longDiv(z, x, y)
})
}
fv, _ := intFloat(f, v)

This comment has been minimized.

@trotterdylan

trotterdylan Mar 22, 2017

Collaborator

You should be able to return NotImplemented in this case and it will delegate to w's operator.

@@ -384,6 +400,7 @@ func initIntType(dict map[string]*Object) {
IntType.slots.Div = &binaryOpSlot{intDiv}
IntType.slots.DivMod = &binaryOpSlot{intDivMod}
IntType.slots.Eq = &binaryOpSlot{intEq}
IntType.slots.FloorDiv = &binaryOpSlot{intFloorDiv}

This comment has been minimized.

@trotterdylan

trotterdylan Mar 22, 2017

Collaborator

Thinking about it a little more, I think you can just use intDiv for this slot. I think it does exactly what you want.

@@ -1,33 +1,405 @@
def eq(a, b):
return a == b
"""

This comment has been minimized.

@trotterdylan

trotterdylan Mar 22, 2017

Collaborator

Seems like this change belongs in a different PR

@corona10
Copy link
Contributor Author

corona10 commented Mar 22, 2017

@trotterdylan Thanks for the early review. I will change them as your comment.

@corona10 corona10 changed the title [WIP] Import modified ouroboros's operator and implement FloorDiv [WIP] Implement floordiv Mar 22, 2017
@corona10 corona10 changed the title [WIP] Implement floordiv [WIP] Implement floordiv and NotImplemented.__repr__() Mar 22, 2017
@corona10 corona10 changed the title [WIP] Implement floordiv and NotImplemented.__repr__() Implement floordiv and NotImplemented.__repr__() Mar 23, 2017
@corona10
Copy link
Contributor Author

corona10 commented Mar 23, 2017

@trotterdylan Ready for review. PTAL

Copy link
Collaborator

trotterdylan left a comment

This is great! Thanks for working on this. Just one question in line.

@@ -471,6 +473,11 @@ func intCheckedDiv(v, w int) (int, divModResult) {
return q, r
}

func intCheckedFloorDiv(v, w int) (int, divModResult) {

This comment has been minimized.

@trotterdylan

trotterdylan Mar 23, 2017

Collaborator

Is this still used somewhere?

This comment has been minimized.

@corona10

corona10 Mar 23, 2017

Author Contributor

@trotterdylan
Not at all. Thanks, I didn't find that! I will update it

@corona10
Copy link
Contributor Author

corona10 commented Mar 23, 2017

Copy link
Collaborator

trotterdylan left a comment

Looks great! Thanks for working on this. Merging.

@trotterdylan trotterdylan merged commit 53a6dfb into google:master Mar 23, 2017
2 checks passed
2 checks passed
cla/google All necessary CLAs are signed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@corona10 corona10 deleted the corona10:operator branch Mar 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.