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

ProcessOutput #233

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

ProcessOutput #233

wants to merge 1 commit into from

Conversation

saltzberg
Copy link

About 2x faster at reading stat files and can parse stat files based on a list of score criteria.

Support for statv1

About 2x faster at reading stat files and can parse stat files based on a list of score criteria.  

Support for statv1
@benmwebb benmwebb self-requested a review April 19, 2017 22:21
Copy link
Member

@benmwebb benmwebb left a comment

Choose a reason for hiding this comment

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

Looks generally OK, and seems like the tests all pass now, so I'll be happy to merge this once you've made the changes I suggested. Oh, and run the file through tools/dev_tools/python_tools/reindent.py or autopep8. The indentation looks a little wonky to me in places.


#Store these keys in a dictionary. Example pair: {109 :'Total_Score'}
self.dict = self.parse_line(line, header=True)
#self.dict = ast.literal_eval(line)
Copy link
Member

Choose a reason for hiding this comment

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

Remove, not comment out, old stuff. If we want to go back, that's what git is for.

for k in kkeys:
self.inv_dict.update({self.dict[k]: k})
else:
print("WARNING: statfile v1 is deprecated. Please convert to statfile v2")
Copy link
Member

Choose a reason for hiding this comment

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

Nothing wrong with your patch here, but when you update it, you should use the "proper" deprecation function, as in current develop.

f.close()


def parse_line(self, line, header=False):
# Parses a line and returns a dictionary of key:value pairs
Copy link
Member

Choose a reason for hiding this comment

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

Use a docstring ("foo") rather than comment (#foo) here so it gets picked up by doxygen. (If you don't want it to be part of the public interface - which requires documentation - make it a private function by calling it _parse_line rather than parse_line.)

fd = split[i]
fields = fd.split(",") # split via commas to get key:value pair
for h in fields[0:-1]:
if h != "": # For some reason, there is occasionally an empty field. Ignoring these seems to work.
Copy link
Member

Choose a reason for hiding this comment

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

Don't duplicate all this code from above. Put it in a function.

def get_keys(self):
""" Returns a list of the string keys that are included in this dictionary
"""
self.klist = [k[1]
Copy link
Member

Choose a reason for hiding this comment

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

Do this only once, not each time the function is called. Easiest would just be to setup klist when the file is first parsed.

# otherwise, just return the string
try:
float(c)
except:
Copy link
Member

Choose a reason for hiding this comment

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

no bare except:



def does_line_pass_criteria(self, fields, c):
# Given a stat file line (as a dictionary) and a criteria tuple, decide whether
Copy link
Member

Choose a reason for hiding this comment

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

docstring



comparison = c[2]
if comparison not in ["==", "<", ">"]:
Copy link
Member

Choose a reason for hiding this comment

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

Generally considered better style to use () rather than [] here, since the set of items is immutable.


comparison = c[2]
if comparison not in ["==", "<", ">"]:
raise Exception('ERROR: IMP.pmi.output.ProcessOutput.does_line_pass_criteria() - Comparison string must be \'>\', \'<\' or \'==\', instead of %s' % (comparison))
Copy link
Member

Choose a reason for hiding this comment

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

Raise a more specific exception. ValueError is probably the most appropriate here.

model_value = self._float_string(fields[intkey])

if type(value) is not type(model_value):
raise Exception('ERROR: IMP.pmi.output.ProcessOutput.does_line_pass_criteria() - Comparison field %s is of type %s while you tried to compare it to a %s' % (key, type(model_value), type(value)))
Copy link
Member

Choose a reason for hiding this comment

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

more specific exception

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.

2 participants