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

Modified insertions and joins for empty matrix #10815

Closed
wants to merge 81 commits into from

Conversation

aravindkanna
Copy link
Contributor

This solves the issue 10770. I think this is the best and simplest way to solve this. This just returns the argument matrix which we want to insert or join irrespective of the position which is the desired thing.

asmeurer and others added 11 commits February 16, 2016 15:38
Previously it was converted to 1 due to logic that was guarding against None
from the slice.
Fixes sympy#10606.

The logic in Range._intersect for intersecting with intervals is particularly
nasty, because of the corner cases with infinite ranges (ranges with infinite
starts are handled differently). I believe the test cases test all the proper
combinations, but the logic should still probably be checked, and it likely
can be simplified.

This changes the behavior of Range.__iter__ to raise an exception for ranges
with infinite starts, since ranges always iterate in order. See the discussion
on the issue for more information about this. I felt that it was better to
always iterate in order, and catch possible bugs if the range is reversed,
than to iterate backwards in this case. It's worth noting, however, that this
makes it more annoying to determine the values in an infinite range beyond the
two that are printed by the pretty printer. But to fix this, I think we should
add better support for slicing ranges.

Also added a better docstring to Range.
It was doing the wrong thing for infinite and reversed ranges.
@smichr
Copy link
Member

smichr commented Mar 13, 2016

This should reference #10781. A better design issue there is that the same type of matrix is returned as was represented by the empty matrix at the start.

@aravindkanna
Copy link
Contributor Author

@smichr

Sir, I didn't get you. Can you please elaborate?

@@ -2082,6 +2084,9 @@ def test_row_insert():
l = [1, 0, 0]
l.insert(i, 4)
assert flatten(eye(3).row_insert(i, r4).col(0).tolist()) == l
a = Matrix([1, 2])
assert Matrix([]).row_insert(10, a) == a
Copy link
Member

Choose a reason for hiding this comment

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

I think for safety reasons, this should raise an error. If someone thinks they are working with a matrix that has size and they aren't, the error should not pass silently. Though a case could be made that list slices that contain out of range bounds are silently truncated as in L = [0, 1, 2]; L[:10] = [42] -> L == [42]. Do you see any merit in either approach?

@aravindkanna
Copy link
Contributor Author

Sir, why is it saying checks have failed. I have checked on my system before committing. Then if there are any errors, I think it would say DO NOT COMMIT. But it didn't say anything. So I have committed. What can I do now Sir?

@jksuom
Copy link
Member

jksuom commented Mar 14, 2016

The checks have failed because the support for Python versions 2.6 and 3.2 was dropped a few days ago.
You should probably pull and merge a new version of master into your branch.

@aravindkanna
Copy link
Contributor Author

Thank you, @jksuom . Have done the same 😄

@@ -3983,6 +3983,8 @@ def row_join(self, rhs):
row
col_join
"""
if not self:
return rhs
Copy link
Member

Choose a reason for hiding this comment

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

here and below, this should be return self.func(rhs) (or similar) so the same type of matrix represented by self is returned. But I think the other duplicate PR will be doing this, so let's see.

@aravindkanna
Copy link
Contributor Author

@smichr
I have made changes to the necessary functions in matrices.py similarly.

if not self:
    typ = type(self)
    return typ(otherargument)

Then what I have found is that I also have to change sparse.py for sparse matrices. So I changed that too. Then I have checked for every combination. It is returning the type of matrix on which we apply the function.

Thanks for the suggestion 👍 😄

jtnydv25 and others added 27 commits March 18, 2016 17:43
Removed trailing whitespaces.
Intersection.reduce short circuits on EmptySet() so
Range(oo).intersect(S.EmptySet) -> S.EmptySet and happens
outside of the Range methods. So rather than sometimes returning
Range(0) and sometimes EmptySet, all Range intersections that
are empty are returned as EmptySet. Slicing of the Range, however,
can return Range(0) (which, unlike xrange/range, does not retain
any information about how the empty Range was obtained).

Range/Range intersections are now defined.

Note about infinite values.
---------------------------

Although selection of an infinite value
in a Range is allowed, e.g. Range(oo)[-1] -> oo, slicing done from
the end containing the infinite value is disallowed since

1) there is no way to count the nth value from oo, e.g. Range(oo)[-2:]
2) there is no way to represent a single infinite value while
retaining normal Range semantics, Range(oo, oo + 1, 1) -> Range(oo, oo, 1)
which would be the empty set. So although Range(oo)[-1:] could be
represented as FiniteSet(oo) or an infinite step could be used and
defined to give only the first value in a Range, e.g. Range(3,oo,oo) ->
Range(3, 4, 1) hence list(Range(oo, 0, -oo)) -> [oo], neither of these
seems like a good idea.

As oo in S.Integers is False, oo in Range(oo) is also False; there
is a slight iconsistency in letting Range(oo)[-1] give oo even though
oo is not in the Range, but I don't see a reason to complicate the
handling of inspection of the Range start/stop values.
Conflicts:
	sympy/printing/pretty/tests/test_pretty.py
Conflicts:
	sympy/printing/latex.py
	sympy/printing/pretty/pretty.py
	sympy/sets/fancysets.py
	sympy/sets/tests/test_fancysets.py
Added algorithm description
Reversed Range and Range.__getitem__
Modified algorithm description
Updated algorithm description
Improved spacing.
Fixed issue sympy#7021 (Faster primepi function)
added testcases
@aravindkanna
Copy link
Contributor Author

Sir, I don't know what is happening. I am creating a new PR for only the modification of sparse.py.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate PR: author's turn The PR has been reviewed and the author needs to submit more changes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet