PyLops / pylops

PyLops – A Linear-Operator Library for Python
https://pylops.readthedocs.io
GNU Lesser General Public License v3.0
431 stars 102 forks source link

Made LinearOperator independent of spLinearOperator #482

Closed rohanbabbar04 closed 1 year ago

rohanbabbar04 commented 1 year ago

For issue #328

Still need to change/update the docs, if code seems fine...

mrava87 commented 1 year ago

Thank you!

I'll try to review this in coming days and let you know :)

rohanbabbar04 commented 1 year ago

Thank You @mrava87 for reviewing my PR, I have updated the docs and agree with all your changes. Do take a look and give your opinion

mrava87 commented 1 year ago

Thank You @mrava87 for reviewing my PR, I have updated the docs and agree with all your changes. Do take a look and give your opinion

Will do soon :)

mrava87 commented 1 year ago

I just identified a problem with describe due to some of the internal linop methods being moved from scipy to native pylops ones...

For example:

A = pylops.MatrixMult(np.ones((10, 5)))
A.name = "A"
C = pylops.MatrixMult(np.ones((10, 5)))

# Sum
D = A + C
describe(D)

now returns A where: {'A': '_SumLinearOperator'} instead of A+M where: {'A': 'MatrixMult', 'M': 'MatrixMult'}.

Currently investigating why...

rohanbabbar04 commented 1 year ago

Is the bug fixed?

rohanbabbar04 commented 1 year ago

Just tested the code Now the describe works fine

A + M
where: {'A': 'MatrixMult', 'M': 'MatrixMult'}
rohanbabbar04 commented 1 year ago

Just tested the code Now the describe works fine

A + M
where: {'A': 'MatrixMult', 'M': 'MatrixMult'}

So I am removing the docstrings to make the Codacy Static Code Analysis successful Removing this and making a commit

mrava87 commented 1 year ago

Yes the code is fixed :)

rohanbabbar04 commented 1 year ago

Yes the code is fixed :)

@mrava87 Made the commit all checks are now successful...

mrava87 commented 1 year ago

@rohanbabbar04 I am satisfied with the current status of the PR.

Since we are making changes to a core method of PyLops, I want to get the feedback of another core developer before merging.. please be patient, we will let you know soon when this is ready to go :)

cako commented 1 year ago

I'll be trying this patch very soon! Sorry for the delays!