Dev analysis #25
Dev analysis #25
Conversation
|
From the assignment sheet: I could not find any user-defined exceptions, I'm willing to admit that I might have missed them. |
quickscreen/analysis/datafill.py
Outdated
| try: | ||
| assert (isinstance(column, int) or column in self.data) | ||
| return self.data.dtypes[column] | ||
| except: |
There was a problem hiding this comment.
Its generally frowned upon to not catch specific errors.
Although this try and except block is simple, having try excepts without specific errors can hide trivial bugs easily.
Also, lab4.md say At least six methods will contain appropriate error and exception handlers which in my reading is catching specific errors.
There was a problem hiding this comment.
adding to Mitch's comments, you can just change your code to be except AssertError, or you can create your own user defined error class and do something along the lines of if not is instance(x, y) raise UserErrorClass() then you can use except UserErrorClass:
| return self.data.dtypes[column] | ||
| except: | ||
| print("Please enter an integer or the name of a column") | ||
| return |
There was a problem hiding this comment.
technically I don't think a return statement is needed on line 67
| return DataEdit(self.data.append(other, ignore_index=True)) | ||
| except: | ||
| print("Not a Pandas data frame.") | ||
| return |
There was a problem hiding this comment.
return statement not needed on 92
| return DataEdit(common) | ||
| print("Not a Pandas data frame.") | ||
| return | ||
|
|
||
|
|
There was a problem hiding this comment.
same comment about return and nested try statements are maybe not ideal - is the inner one needed?
There was a problem hiding this comment.
yeah, this seems a little funky
I think it could be replaced with
try:
statement0
except ExceptionTypeOne:
statement1
except ExceptionTypeTwo:
statement2
| assert isinstance(self.data, pd.core.frame.DataFrame) | ||
| except: | ||
| print("Not a Pandas data frame.") | ||
| return |
|
looks good to me now, tbh I didn't really review it in great detail just skimmed it and it looked like its all good now |
I couldn't get Datafill to 100% coverage its at 95% right now. I think Debangsha said in the lab he wasn't going to be a stickler for exactly 100%. Linear has trouble getting high coverage due to the lines of code that create graphs.