MAINT: Removed suitable unused variables shown in LGTM by ajayd-san · Pull Request #19102 · numpy/numpy

ajayd-san · 2021-05-26T02:05:25Z

Fixes most of the unused variable linting errors shown in LGTM.

partially closes #19077

mattip · 2021-05-26T05:04:27Z

numpy/f2py/crackfortran.py

                if v[0] == '(' and v[-1] == ')':
-                    # FIXME, unused l looks like potential bug
-                    l = markoutercomma(v[1:-1]).split('@,@')
+                    pass


Does this reflect a deeper problem? Should the result of markeroutercomma be used somewhere?

I think this should be left for a future clean-up PR.

This looks like an unfinished implementation. In short term, replace the block under elif iscomplex(...): with

outmess(f'get_parameters[TODO]: implement evaluation of complex expression {v}')

Looks like this is resolved.

numpy/f2py/crackfortran.py

mattip · 2021-05-26T05:22:28Z

Unless one of the f2py people can approve that the unused function call results are indeed not needed, I would suggest removing them for now. @pearu, @melissawm thoughts?

ajayd-san · 2021-05-26T05:44:55Z

sure, I'll follow up with a new commit shortly.

…ted in LGTM

numpy/lib/format.py

numpy/ma/mrecords.py

eric-wieser · 2021-05-26T08:08:53Z

numpy/distutils/system_info.py

        conf = c()
        conf.verbosity = 2
-        # FIXME: r not used
-        r = conf.get_info()


I suspect this has side effects based on the verbosity settings above, so the conf.get_info call is still needed

sure, I'll reflect that change in a new commit.

…o.py

pearu

My review covers only the crackfortran.py part.

pearu · 2021-05-26T10:27:31Z

numpy/f2py/crackfortran.py

                        (m.group('this'), ll[:i]))
                ll = ll + ','.join(groupcache[groupcounter]['args'])
        if i < 0:
-            i = 0


This is valid removal.

hey, I commited your reviewed changes. Can you tell if you are satisfied ?
The azure lint test seems to fail, reason being outmess(f'get_parameters[TODO]: implement evaluation of complex expression {v}'), Can you check?

That's a "line too long" error. You'll need to split the line, for instance:

outmess('get_parameters[TODO]:' f' implement evaluation of complex expression {v}')

(untested).

The removal looks to be gone, wasn't it valid?

pearu · 2021-05-26T10:34:50Z

numpy/f2py/crackfortran.py

                if v[0] == '(' and v[-1] == ')':
-                    # FIXME, unused l looks like potential bug
-                    l = markoutercomma(v[1:-1]).split('@,@')
+                    pass


This looks like an unfinished implementation. In short term, replace the block under elif iscomplex(...): with

outmess(f'get_parameters[TODO]: implement evaluation of complex expression {v}')

numpy/f2py/crackfortran.py

… in crackfortran.py after review

numpy/distutils/system_info.py

numpy/linalg/linalg.py

numpy/ma/core.py

… added in previous commit

…py - fixed Azure linter 'line too long' error

…ved '_linalgRealType(t)' function from linalg.py

charris · 2021-05-28T00:02:32Z

There test failure is unrelated.

charris · 2021-06-10T22:20:42Z

numpy/f2py/crackfortran.py

-                    # FIXME, unused l looks like potential bug
-                    l = markoutercomma(v[1:-1]).split('@,@')
+                outmess(f'get_parameters[TODO]:'
+                 f'implement evaluation of complex expression {v}')


Indentation is off, should align under first string.

eric-wieser · 2021-06-10T22:33:08Z

numpy/distutils/system_info.py

-        # FIXME: r not used
+        # we don't need the result, but we want
+        # the side effect of printing diagnostics
        r = conf.get_info()


Suggested change

r = conf.get_info()

conf.get_info()

…les in system_info.py

ajayd-san · 2021-06-11T07:04:07Z

can anyone review changes ?

eric-wieser · 2021-06-11T07:25:09Z

numpy/f2py/crackfortran.py

-                if v[0] == '(' and v[-1] == ')':
-                    # FIXME, unused l looks like potential bug
-                    l = markoutercomma(v[1:-1]).split('@,@')
+                outmess(f'get_parameters[TODO]:'


Suggested change

outmess(f'get_parameters[TODO]:'

outmess(f'get_parameters[TODO]: '

Else this will print TODO]:implement with no space

eric-wieser · 2021-06-11T07:26:02Z

numpy/f2py/crackfortran.py

 def analyzeargs(block):
    setmesstext(block)
-    implicitrules, attrrules = buildimplicitrules(block)
+    implicitrules = buildimplicitrules(block)[0]


Suggested change

implicitrules = buildimplicitrules(block)[0]

implicitrules, _ = buildimplicitrules(block)

Is probably slightly clearer

ajayd-san · 2021-06-11T13:20:46Z

C:\hostedtoolcache\windows\Python\3.9.5\x86\lib\site-packages\numpy\core\tests\test_multiarray.py:3029: RuntimeWarning

this seems to be coming from multiarray.py but i didn't make any changes to that file.
Can anyone review ?

seberg · 2021-07-08T22:26:20Z

Everything besides the f2py seems pretty simple, and I guess Pearu is happy with that. @charris happy to merge?

pearu

LGTM

mattip · 2021-07-09T06:06:15Z

Thanks @default-303

MAINT: Removed suitable unused variables shown in LGTM

115ed1d

github-actions bot added the 03 - Maintenance label May 26, 2021

mattip reviewed May 26, 2021

View reviewed changes

numpy/f2py/crackfortran.py Show resolved Hide resolved

MAINT: Restored crackfortran.py after review for unused variables lis…

3e2c20b

…ted in LGTM

BvB93 reviewed May 26, 2021

View reviewed changes

numpy/lib/format.py Outdated Show resolved Hide resolved

BvB93 reviewed May 26, 2021

View reviewed changes

numpy/ma/mrecords.py Show resolved Hide resolved

eric-wieser reviewed May 26, 2021

View reviewed changes

MAINT: Restored variables related to verbosity settings in system_inf…

2028529

…o.py

pearu reviewed May 26, 2021

View reviewed changes

MAINT: Removed unused variables and handled unfinished implementation…

952c519

… in crackfortran.py after review

eric-wieser reviewed May 26, 2021

View reviewed changes

numpy/distutils/system_info.py Outdated Show resolved Hide resolved

eric-wieser reviewed May 26, 2021

View reviewed changes

numpy/linalg/linalg.py Show resolved Hide resolved

eric-wieser reviewed May 26, 2021

View reviewed changes

numpy/ma/core.py Show resolved Hide resolved

ajayd-san added 4 commits May 26, 2021 17:27

MAINT: Addressed Azure lint check 'Line too long error' on line 2554,…

366872c

… added in previous commit

MAINT: Added comments for future refrence in core.py and system_info.py

df363b0

MAINT: Added comments for future refrence in core.py and system_info.…

b30318a

…py - fixed Azure linter 'line too long' error

MAINT: Removed 'if' block which served no purpose in core.py and remo…

beacb39

…ved '_linalgRealType(t)' function from linalg.py

charris reviewed Jun 10, 2021

View reviewed changes

eric-wieser mentioned this pull request Jun 10, 2021

removed some unused imports and variables. #19222

Closed

eric-wieser reviewed Jun 10, 2021

View reviewed changes

MAINT: fixed indentation in crackfortran.py and removed unused variab…

c76b763

…les in system_info.py

eric-wieser reviewed Jun 11, 2021

View reviewed changes

MAINT: added space for readability in strings and basic refactoring

6ce7835

charris mentioned this pull request Jun 25, 2021

Maintenance/1.21.x #19333

Closed

pearu approved these changes Jul 9, 2021

View reviewed changes

mattip merged commit 1f95d79 into numpy:main Jul 9, 2021

seberg mentioned this pull request Nov 14, 2021

BUG: removal of mask copy code may have broken matplotlib test #20372

Closed

greglucas mentioned this pull request Nov 16, 2021

REV: Add MaskedArray creation from non nd-array back in #20386

Merged

WarrenWeckesser mentioned this pull request Nov 18, 2021

Possible bug in numpy/ma/core.py #6045

Closed

	outmess(f'get_parameters[TODO]:'
	outmess(f'get_parameters[TODO]: '

	implicitrules = buildimplicitrules(block)[0]
	implicitrules, _ = buildimplicitrules(block)

Uh oh!

Conversation

ajayd-san commented May 26, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mattip commented May 26, 2021

Uh oh!

ajayd-san commented May 26, 2021

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pearu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

charris commented May 28, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ajayd-san commented Jun 11, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ajayd-san commented Jun 11, 2021

Uh oh!

seberg commented Jul 8, 2021

Uh oh!

pearu left a comment

Choose a reason for hiding this comment

Uh oh!

mattip commented Jul 9, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants