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

Amended too long lines in conditionset.py #12491

Merged
merged 2 commits into from
Sep 23, 2017
Merged

Amended too long lines in conditionset.py #12491

merged 2 commits into from
Sep 23, 2017

Conversation

jremes-foss
Copy link
Contributor

Hi.

I rectified couple of issues with file sympy/sets/conditionset.py:

  1. There were several too long lines. These have been now amended.
  2. There was unused import in one of the examples. This has been now removed. Example was tested locally after removal of unused import - fully functional.

./bin/test quality executed locally - all green.

Thank you.

@@ -39,10 +39,20 @@ def __new__(cls, sym, condition, base_set):
if isinstance(base_set, EmptySet):
return base_set
if isinstance(base_set, FiniteSet):
sifted = sift(base_set, lambda _: fuzzy_bool(condition.subs(sym, _)))
Copy link
Member

Choose a reason for hiding this comment

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

We tend to not prettify these too much. How about the following?

                sifted = sift(base_set, lambda _: fuzzy_bool(
                    condition.subs(sym, _)))
                return Union(FiniteSet(*sifted[True]), Basic.__new__(
                    cls, sym, condition, FiniteSet(*sifted[None])))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi. Sure, I will fix this.

@debugger22
Copy link
Member

@Newman101 Maybe this can be merged once you change what was suggested.

@jremes-foss
Copy link
Contributor Author

Sure, I will do these next.

@jremes-foss
Copy link
Contributor Author

Took a while but it's done now - both tests ran locally, all green.

@gxyd
Copy link
Contributor

gxyd commented Sep 23, 2017

I had restarted the failing test, it has passed now. Thanks, I am merging this.

@gxyd gxyd merged commit 31af3c4 into sympy:master Sep 23, 2017
@jremes-foss
Copy link
Contributor Author

Good to know, thanks!

@jremes-foss jremes-foss deleted the conditionset_too_long_line branch September 28, 2017 13:56
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.

4 participants