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

Set literal #8959

Merged
merged 1 commit into from Nov 29, 2015
Merged

Set literal #8959

merged 1 commit into from Nov 29, 2015

Conversation

remyleone
Copy link
Contributor

No description provided.

@@ -115,8 +115,8 @@ def test_sets():
"""
Test that set and frozenset use Python 3 formatting.
"""
objects = [set(), frozenset(), set([1]), frozenset([1]), set([1, 2]),
Copy link
Member

Choose a reason for hiding this comment

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

so here the non literal seem to be on purpose: Test that set and frozenset use Python 3 formatting
Otherwise, we would have tested the round-tripping of the string representation through eval.

@Carreau
Copy link
Member

Carreau commented Nov 9, 2015

In general we try to avoid huge cleanup PRs, this one is not too big, so it's fine.
One of the reason is that there is often the possibility to introduce changes to things that are made on purpose (see here example of frozenset), and also because big cleanup that touch many file, make it much harder to git blame, or bisect a specific part of code.

@minrk minrk changed the title Set litteral Set literal Nov 9, 2015
@remyleone
Copy link
Contributor Author

@Carreau Fixed

@remyleone
Copy link
Contributor Author

@Carreau Are you still interested or should it be closed?

@Carreau
Copy link
Member

Carreau commented Nov 29, 2015

Yes, sorry. Thanks !

Carreau added a commit that referenced this pull request Nov 29, 2015
@Carreau Carreau merged commit 7098025 into ipython:master Nov 29, 2015
@minrk minrk modified the milestone: 4.1 Dec 14, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants