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

Fix min/max functions with generators, and 'None' default by gazpachoking · Pull Request #514 · PythonCharmers/python-future · GitHub

/ python-future Public

Fix min/max functions with generators, and 'None' default - #514

Merged
jmadler merged 5 commits into
PythonCharmers:masterfrom
gazpachoking:fix_min_max
Oct 24, 2019
ConversationCommits (5)ChecksFiles changed
Merged

Fix min/max functions with generators, and 'None' default#514
jmadler merged 5 commits into
PythonCharmers:masterfrom
gazpachoking:fix_min_max

Conversation

gazpachoking commented Oct 17, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

I allowed a few fixes to creep into this PR, I can separate them out if desired. This PR addresses:

  • There is an issue with the min/max functions consuming the first argument of iterators that cannot be looped over more than once. max() over itertools.ifilter() is broken in 0.18.0 #510
  • The new min/max functions did not allow None as a default.
  • Aligned the error message when an empty sequence was passed to the new min/max functions to be the same as the builtin min/max error message.

gazpachoking commented Oct 17, 2019

Copy link
Copy Markdown
Contributor Author

Oops, didn't see we already had some tests in test_builtins. I'll just add the appropriate new tests there.

gazpachoking marked this pull request as ready for review October 18, 2019 01:52

gazpachoking commented Oct 18, 2019

Copy link
Copy Markdown
Contributor Author

A couple more changes crept into this PR, updated the original PR text to reflect this.

gazpachoking changed the title Fix min/max functions with generators Fix min/max functions with generators, and 'None' default Oct 18, 2019

gazpachoking commented Oct 18, 2019

Copy link
Copy Markdown
Contributor Author

Not sure why travis isn't adding the results here, pretty sure the 2.6 failures have nothing to do with this PR though. https://travis-ci.org/PythonCharmers/python-future/builds/599676065

megies mentioned this pull request Oct 21, 2019
Closed

megies commented Oct 21, 2019

Copy link
Copy Markdown
Contributor

I can confirm this fixes an issue for me which looks like what is described in #510 first post, see #510 (comment).

I recommend merging and releasing this

megies mentioned this pull request Oct 21, 2019
Closed
jmadler merged commit 228a297 into PythonCharmers:master Oct 24, 2019
gazpachoking deleted the fix_min_max branch December 6, 2019 04:28
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL