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

Recommended lgtm python issues #46

Closed
avan989 opened this issue Dec 20, 2019 · 9 comments · Fixed by #96 or #109
Closed

Recommended lgtm python issues #46

avan989 opened this issue Dec 20, 2019 · 9 comments · Fixed by #96 or #109
Assignees
Labels
bug Something isn't working
Milestone

Comments

@avan989
Copy link
Contributor

avan989 commented Dec 20, 2019

Is your feature request related to a problem? Please describe.
Recommendation from lgtm:

CHeaderParser.py

else:
--
325 | line_num += 1
326 | pass
  | Unnecessary 'pass' statement.

TlmMQRecv.py

# We never get here but clean up anyhow
--
42 | subscriber.close()
  | Unreachable statement.
43 |  
RoutingService.py

self.sock.bind(('', udpRecvPort))
--
  | '' binds a socket to all interfaces.

CommandParser.py

line 20:

import sys
--
  | Import of 'sys' is not used.
21 | import csv
  | Import of 'csv' is not used.
22 | import getopt
  | Import of 'getopt' is not used.
23 | import subprocess
  | Import of 'subprocess' is not used.
24 | import shlex
  | Import of 'shlex' is not used.

HTMLDocsParser.py

line 20:

import sys
--
  | Import of 'sys' is not used.
21 | import csv
  | Import of 'csv' is not used.
22 | import getopt
  | Import of 'getopt' is not used.
23 | import subprocess
  | Import of 'subprocess' is not used.
24 | import shlex
  | Import of 'shlex' is not used.


ParameterDialog.py

line 234:
def retranslateUi(self, Dialog):
--
  | All statements in retranslateUi are similar in 4 Values.

GroundSystem.py

line 26:
import socket
--
  | Import of 'socket' is not used.
27 | import zmq
  | Import of 'zmq' is not used.
28 |  
29 | from PyQt4 import QtGui, QtNetwork, QtCore
  | Import of 'QtCore' is not used.Import of 'QtNetwork' is not used.


# Start FDL-FUL gui system
98 | #def startFDLSystem(self):
99 | #    selectedSpacecraft = self.getSelectedSpacecraftName()
100 | #    if selectedSpacecraft == 'All':
101 | #        subscription = ''
102 | #        self.DisplayErrorMessage('Cannot open FDL manager.\nNo spacecraft selected.')
103 | #    else:
104 | #       subscription = '--sub=GroundSystem.' + selectedSpacecraft
105 | #       os.system('( cd Subsystems/fdlGui/ && python FdlSystem.py ' + subscription + ' ) & ')
  | These comments appear to contain commented-out code.
106 |  

RoutingService.py

line 23:
import sys
--
  | Import of 'sys' is not used.
24 | import os
  | Import of 'os' is not used.
25 | import socket
26 | import zmq
27 |  
28 | from PyQt4 import QtGui, QtNetwork, QtCore
  | Import of 'QtGui' is not used.Import of 'QtNetwork' is not used.

Paramete.py

import csv
--
  | Import of 'csv' is not used.


from PyQt4 import QtGui, QtNetwork
--
  | Import of 'QtNetwork' is not used.
29 | from ParameterDialog import Ui_Dialog
30 | from HTMLParser import HTMLParser
  | Import of 'HTMLParser' is not used.


line 104:
idx = 0
--
  | The global variable 'idx' is not used.


line 111:
params = ''
--
  | The global variable 'params' is not used.

line 125:
idx = int(arg) # comand index in command definition file
--
  | The global variable 'idx' is not used.

line 163, 169, 174, 179, 184, 189, 194, 199, 204, 209
'except' clause does nothing but pass and there is no explanatory comment.

UdpCommands.py

import csv
--
  | Import of 'csv' is not used.

from HTMLParser import HTMLParser
--
  | Import of 'HTMLParser' is not used.

CHeaderParser.py

import sys, os
--
  | Import of 'os' is not used.

line 437:
typeNew = parser.findKeyword(dataTypesOrig[-1])
--
  | The global variable 'typeNew' is not used.


except IOError as e:
--
115 | print "Couldn't find default file. Checking command line arguments."
116 | except:
  | Except block directly handles BaseException.


array_size_within_limit = True
--
489 | # End of the loop.
490 | except:
  | Except block directly handles BaseException.


#else:
--
188 | #       print "Did not find any comments in definition."
  | These comments appear to contain commented-out code.


GenericTelemetry.py

import pdb
--
  | Import of 'pdb' is not used.


class TlmReceiver(QtCore.QThread):
--
127 |  
128 | def __init__(self, mainWindow, subscription):
  | All 6 statements in __init__ are identical in __init__.


TlmUDPSender.py

import sys
--
  | Import of 'sys' is not used.

EventMessage.py

line 151 - 153
udpPort  = 10000
--
  | The global variable 'udpPort' is not used.
152 | appId = 999
153 | tlmDefFile = "not-needed.txt"
  | The global variable 'tlmDefFile' is not used.

line 171:
udpPort = arg
--
  | The global variable 'udpPort' is not used.

line 175:
tlmDefFile = arg
--
  | The global variable 'tlmDefFile' is not used.


line 193 - 195:
py_endian = '<'
--
  | The global variable 'py_endian' is not used.

py_endian = '>'
--
  | The global variable 'py_endian' is not used.

GenericTelemetry.py

line 166:
udpPort  = 10000
--
  | The global variable 'udpPort' is not used.

line 186:
udpPort = arg
--
  | The global variable 'udpPort' is not used.

line 252:
for j in range (i, 40):
--
  | The global variable 'j' is not used.

UdpCommands.py

line 247:
for i in range(len(cmdDesc)):
--
  | The global variable 'i' is not used.

line 249:
for i in range (len(cmdDesc), 26):
--
  | The global variable 'i' is not used.

CommandSystem.py

for j in range (i, 22):
--
  | The global variable 'j' is not used.

line 341, 355, 369, 383, 397, 411, 425, 439, 453, 467, 481, 495, 509, 523, 537, 551, 565, 579, 593, 607, 621, 635

'except' clause does nothing but pass and there is no explanatory comment.

TelemetrySystem.py


#
--
71 | def strToHex(aString):
  | Normal methods should have 'self', rather than 'aString', as their first parameter.


#
--
80 | def dumpPacket(packetData):
  | Normal methods should have 'self', rather than 'packetData', as their first parameter.



line 82:
appIdString = appIdString + "%02X" % ord(packetData[1])
--
  | The value assigned to local variable 'appIdString' is never used.

line 167:
def dumpPacket(packetData):
--
  | The value assigned to local variable 'dumpPacket' is never used.

line 192:
send_host = "127.0.0.1"
--
  | The value assigned to local variable 'send_host' is never used.

193 | send_port =  tlmPagePort[i]
  | The value assigned to local variable 'send_port' is never used.


line 249:
def __init__(self, mainWindow, subscription):
--
  | All 6 statements in __init__ are identical in __init__.

Requester Info
Anh Van, NASA Goddard

@skliper skliper added the enhancement New feature or request label Mar 16, 2020
@lbleier-GSFC
Copy link
Contributor

This issue has been updated to be Python-related only; C issues are consolidated in issue #43

@skliper skliper changed the title Recommended lgtm issues Recommended lgtm python issues Apr 13, 2020
@lbleier-GSFC lbleier-GSFC self-assigned this May 27, 2020
lbleier-GSFC added a commit to lbleier-GSFC/cFS-GroundSystem that referenced this issue May 27, 2020
Problems shown in this issue were fixed in nasa#72 as part of
updates/refactoring. Other lgtm issues addressed here.
Problems in auto-generated .py files not addressed.
Auto-generated .py files renamed with Ui_ prefix.
The lgtm.yml file must be updated to exclude these
See nasa/cFS#92
@skliper skliper linked a pull request Jun 1, 2020 that will close this issue
lbleier-GSFC added a commit to lbleier-GSFC/cFS-GroundSystem that referenced this issue Jun 24, 2020
Problems shown in this issue were fixed in nasa#72 as part of
updates/refactoring. Other lgtm issues addressed here.
Problems in auto-generated .py files not addressed.
Auto-generated .py files renamed with Ui_ prefix.
The lgtm.yml file must be updated to exclude these
See nasa/cFS#92
@skliper
Copy link
Contributor

skliper commented Jun 24, 2020

@lbleier-GSFC have these been resolved as part of your previous work? Doesn't look like these are listing the autogenerated files,

@skliper
Copy link
Contributor

skliper commented Jun 24, 2020

@lbleier-GSFC see https://lgtm.com/projects/g/nasa/cFS/alerts/?mode=list&lang=python for the latest set of issues. These all look to be against hand generated files. Can they be resolved?

@lbleier-GSFC
Copy link
Contributor

lbleier-GSFC commented Jun 24, 2020

@lbleier-GSFC see https://lgtm.com/projects/g/nasa/cFS/alerts/?mode=list&lang=python for the latest set of issues. These all look to be against hand generated files. Can they be resolved?

I'm very confused. I fixed these already. I am looking at my local copies and they are fixed. Not only that, the auto-generated ones here should be deleted because I changed the filenames (added the Ui_ prefix)

@skliper
Copy link
Contributor

skliper commented Jun 24, 2020

@lbleier-GSFC see https://lgtm.com/projects/g/nasa/cFS/alerts/?mode=list&lang=python for the latest set of issues. These all look to be against hand generated files. Can they be resolved?

I'm very confused. I fixed these already. I am looking at my local copies and they are fixed. Not only that, the auto-generated ones here should be deleted because I changed the filenames (added the Ui_ prefix)

Did you? I just looked in master for the first LGTM alert, and it was still in the code (QtGui not used in CommandSystemDialog.py). Did you submit something that didn't get checked in? What are you looking at locally? Is there a branch you have locally that needs a PR?

@lbleier-GSFC
Copy link
Contributor

lbleier-GSFC commented Jun 25, 2020

@lbleier-GSFC see https://lgtm.com/projects/g/nasa/cFS/alerts/?mode=list&lang=python for the latest set of issues. These all look to be against hand generated files. Can they be resolved?

I'm very confused. I fixed these already. I am looking at my local copies and they are fixed. Not only that, the auto-generated ones here should be deleted because I changed the filenames (added the Ui_ prefix)

Did you? I just looked in master for the first LGTM alert, and it was still in the code (QtGui not used in CommandSystemDialog.py). Did you submit something that didn't get checked in? What are you looking at locally? Is there a branch you have locally that needs a PR?

That particular file is auto-generated. I fixed the alerts for files that were hand-written.

I am looking at the branch I made for this issue

@skliper
Copy link
Contributor

skliper commented Jun 25, 2020

@lbleier-GSFC see https://lgtm.com/projects/g/nasa/cFS/alerts/?mode=list&lang=python for the latest set of issues. These all look to be against hand generated files. Can they be resolved?

I'm very confused. I fixed these already. I am looking at my local copies and they are fixed. Not only that, the auto-generated ones here should be deleted because I changed the filenames (added the Ui_ prefix)

Did you? I just looked in master for the first LGTM alert, and it was still in the code (QtGui not used in CommandSystemDialog.py). Did you submit something that didn't get checked in? What are you looking at locally? Is there a branch you have locally that needs a PR?

That particular file is auto-generated. I fixed the alerts for files that were hand-written.

I am looking at the branch I made for this issue

Copy. Looks like LGTM hasn't run an updated analysis on the latest master yet. I haven't been able to trigger it, but apparently it does a daily pull.

@lbleier-GSFC
Copy link
Contributor

@lbleier-GSFC see https://lgtm.com/projects/g/nasa/cFS/alerts/?mode=list&lang=python for the latest set of issues. These all look to be against hand generated files. Can they be resolved?

I'm very confused. I fixed these already. I am looking at my local copies and they are fixed. Not only that, the auto-generated ones here should be deleted because I changed the filenames (added the Ui_ prefix)

Did you? I just looked in master for the first LGTM alert, and it was still in the code (QtGui not used in CommandSystemDialog.py). Did you submit something that didn't get checked in? What are you looking at locally? Is there a branch you have locally that needs a PR?

That particular file is auto-generated. I fixed the alerts for files that were hand-written.
I am looking at the branch I made for this issue

Copy. Looks like LGTM hasn't run an updated analysis on the latest master yet. I haven't been able to trigger it, but apparently it does a daily pull.

What's odd is that I renamed auto-generated files such as these to have the Ui_ prefix in their filename. The pre-renamed files should be deleted as far as git is concerned. I am not sure why they are still coming up

@skliper
Copy link
Contributor

skliper commented Jun 25, 2020

@lbleier-GSFC coming up where? In LGTM it is because the last run was on a previous commit. You can see the commit it used and compare it with master (it's using the 6/10 IC merge last I checked).

lbleier-GSFC added a commit to lbleier-GSFC/cFS-GroundSystem that referenced this issue Jun 29, 2020
Problems shown in this issue were fixed in nasa#72 as part of
updates/refactoring. Other lgtm issues addressed here.
Problems in auto-generated .py files not addressed.
Auto-generated .py files renamed with Ui_ prefix.
The lgtm.yml file must be updated to exclude these
See nasa/cFS#92
lbleier-GSFC added a commit to lbleier-GSFC/cFS-GroundSystem that referenced this issue Jun 29, 2020
commit 2a8911f
Author: Leor Bleier <[email protected]>
Date:   Fri Jun 26 17:34:54 2020 -0400

    Enhancement nasa#103 - updates to allow user to select header version

    Updates GUI and backend to allow user to select
    header version offsets, or custom byte offsets

commit 9d233eb
Author: Leor Bleier <[email protected]>
Date:   Thu Jun 25 17:12:31 2020 -0400

    WIP: Enhancement nasa#103 - updates to miniCmdUtil and other files

commit 6d3f7ed
Author: Leor Bleier <[email protected]>
Date:   Wed Jun 24 13:53:51 2020 -0400

    WIP: Enhancement nasa#103 - updates to miniCmdUtil

commit 8ad565c
Author: Leor Bleier <[email protected]>
Date:   Wed Jun 24 10:56:45 2020 -0400

    WIP: Enhancement nasa#103 - updates to miniCmdUtil

commit a0d1872
Author: Leor Bleier <[email protected]>
Date:   Tue Jun 23 15:39:25 2020 -0400

    WIP: Enhancement nasa#103 - Implemented native cmdUtil

    Native cmdUtil has only subset of full cmdUtil functionality
    Also implemented updates to GUI and backend to support custom
    byte offsets in tlm and cmd

commit 09261e5
Author: Leor Bleier <[email protected]>
Date:   Tue Jun 16 13:27:26 2020 -0400

    Enhancement nasa#103 - custom header mechanism

    Fix for rebase merge error

commit 7754c9f
Author: Leor Bleier <[email protected]>
Date:   Tue Jun 16 12:38:33 2020 -0400

    Enhancement nasa#103 - custom header mechanism

    Updated UI and backend to support custom header sizes.
    Change only impacts tlm currently

commit 821f06c
Author: Leor Bleier <[email protected]>
Date:   Mon Jun 1 09:57:02 2020 -0400

    Feature nasa#98 - Refactor UI to table widgets

    Removed unnecessary file

commit b0bd3fa
Author: Leor Bleier <[email protected]>
Date:   Mon Jun 1 09:06:45 2020 -0400

    Feature nasa#98 - Refactor UI to use table widgets

    Backend updated accordingly. Other various tweaks/fixes as needed

commit fb0ccb6
Author: Leor Bleier <[email protected]>
Date:   Wed May 27 16:23:40 2020 -0400

    Fix nasa#46 Recommended lgtm python issues

    Problems shown in this issue were fixed in nasa#72 as part of
    updates/refactoring. Other lgtm issues addressed here.
    Problems in auto-generated .py files not addressed.
    Auto-generated .py files renamed with Ui_ prefix.
    The lgtm.yml file must be updated to exclude these
    See nasa/cFS#92

commit b3d8039
Author: Leor Bleier <[email protected]>
Date:   Wed May 20 07:54:39 2020 -0400

    Fix nasa#88 - Modify GroundSystem to use JSON files generated by CCDD

    Also includes further updates and refinements to overall UI

commit 26486cb
Author: Leor Bleier <[email protected]>
Date:   Thu May 14 11:37:11 2020 -0400

    Fix nasa#72 - Upgrade PyQt4 to PyQt5

    Further edits based on CCB feedback, and other fixes as necessary

commit 876a39d
Author: Leor Bleier <[email protected]>
Date:   Mon May 11 17:08:21 2020 -0400

    Fix nasa#72 - Upgrade PyQt4 to PyQt5

    Includes code cleanup/refactoring. Also fixes nasa#71
astrogeco added a commit that referenced this issue Jul 1, 2020
Fix #46 Recommended lgtm python issues
@astrogeco astrogeco added bug Something isn't working and removed enhancement New feature or request labels Oct 1, 2020
@astrogeco astrogeco added this to the 2.2.0 milestone Oct 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
4 participants