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

more itertools defined #15

Merged
merged 7 commits into from Jan 5, 2017
Merged

more itertools defined #15

merged 7 commits into from Jan 5, 2017

Conversation

@AlexEKoren
Copy link
Contributor

@AlexEKoren AlexEKoren commented Jan 4, 2017

Added cycle, dropwhile, from_iterable, ifilter, ifilterfalse, and takewhile in itertools

@googlebot
Copy link

@googlebot googlebot commented Jan 4, 2017

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@AlexEKoren
Copy link
Contributor Author

@AlexEKoren AlexEKoren commented Jan 4, 2017

I signed it!

@googlebot
Copy link

@googlebot googlebot commented Jan 4, 2017

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.

@ns-cweber
Copy link
Contributor

@ns-cweber ns-cweber commented Jan 4, 2017

Might be good to have some test cases as well.

@AlexEKoren
Copy link
Contributor Author

@AlexEKoren AlexEKoren commented Jan 4, 2017

Done! Thanks for the feedback

@ns-cweber
Copy link
Contributor

@ns-cweber ns-cweber commented Jan 4, 2017

Looks like you've committed some .pyc files by accident?

break
assert tuple(got) == want, 'tuple(cycle%s) == %s, want %s' % (arg, tuple(got), want)

def testDropwhile():
Copy link
Contributor

@ns-cweber ns-cweber Jan 4, 2017

Choose a reason for hiding this comment

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

def TestDropwhile? (capitalize)

@AlexEKoren
Copy link
Contributor Author

@AlexEKoren AlexEKoren commented Jan 4, 2017

Good catch. Should be fixed. Changed the .gitignore as well.

@ns-cweber
Copy link
Contributor

@ns-cweber ns-cweber commented Jan 4, 2017

Good work. I couldn't find any more problems. 👍 (apart from the CLA issue that's blocking the PR).

@AlexEKoren
Copy link
Contributor Author

@AlexEKoren AlexEKoren commented Jan 5, 2017

Awesome! I signed the CLA, but not sure why it's not going through. What can I do rectify?

@trotterdylan
Copy link
Collaborator

@trotterdylan trotterdylan commented Jan 5, 2017

Really appreciate you taking this on!

Not sure what's up with the CLA issue, but did you see this comment? #15 (comment)

Do you have multiple emails associated with your GitHub account? If so, then make sure that the email that's associated with the git commits is the same one on the CLA?

@AlexEKoren
Copy link
Contributor Author

@AlexEKoren AlexEKoren commented Jan 5, 2017

Yep, I double checked. Same e-mail (capitalization is different, but Github won't let me change it. Guessing that's not the issue)

@trotterdylan
Copy link
Collaborator

@trotterdylan trotterdylan commented Jan 5, 2017

Strangely, it looks like one of your commits is not like the other:

9sarmkau43u

The one by AlexEKoren is fine, but the others aren't?

@googlebot
Copy link

@googlebot googlebot commented Jan 5, 2017

CLAs look good, thanks!

@AlexEKoren
Copy link
Contributor Author

@AlexEKoren AlexEKoren commented Jan 5, 2017

@trotterdylan my CLI was setup with another e-mail account. Rewrote old commits with correct credentials! Wow, that was a weird one, great catch.

@trotterdylan
Copy link
Collaborator

@trotterdylan trotterdylan commented Jan 5, 2017

Awesome. This all looks great. Can you make sure that "make precommit" is clean?

@AlexEKoren
Copy link
Contributor Author

@AlexEKoren AlexEKoren commented Jan 5, 2017

Just to make sure I understand correctly: Run 'make precommit' in the root directory and make sure it runs without error?

@trotterdylan
Copy link
Collaborator

@trotterdylan trotterdylan commented Jan 5, 2017

Right. The process should exit with status 0 (there may be output though).

@AlexEKoren
Copy link
Contributor Author

@AlexEKoren AlexEKoren commented Jan 5, 2017

I get the following output. Thoughts? Tried to make clean as well.

build/src/grumpy/builtin_types.go:587: z.Text undefined (type big.Int has no field or method Text)
build/src/grumpy/long.go:131: x.Text undefined (type *big.Int has no field or method Text)
build/src/grumpy/long.go:281: toLongUnsafe(o).value.Text undefined (type big.Int has no field or method Text)
build/src/grumpy/long.go:289: toLongUnsafe(o).value.Text undefined (type big.Int has no field or method Text)
make: *** [build/pkg/darwin_amd64/grumpy.a] Error 1

@trotterdylan
Copy link
Collaborator

@trotterdylan trotterdylan commented Jan 5, 2017

That's not a problem with your code. It may be that you have an older Go toolchain. Can you paste the output of "go version"? Mine is:

$ go version
go version go1.7.4 darwin/amd64

@AlexEKoren
Copy link
Contributor Author

@AlexEKoren AlexEKoren commented Jan 5, 2017

Oops! Was a while since I updated Go.

'make precommit' runs with exit status 0!

@trotterdylan trotterdylan merged commit 61bd06c into google:master Jan 5, 2017
2 checks passed
@trotterdylan
Copy link
Collaborator

@trotterdylan trotterdylan commented Jan 5, 2017

Awesome! Thanks for working on this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants