Debug OUTCAR and CHGCAR parsing #185
Conversation
Pull Request Test Coverage Report for Build 1300
💛 - Coveralls |
when confronting empty files
when confronting empty files. Only parse if not empty. Else return None
sudarsan-surendralal
changed the title
|
I guess the file size part is already fixed in #188 |
|
Then can we somehow merge these two requests |
Check file size
| @@ -150,6 +151,8 @@ def _read_vol_data(self, filename, normalize=True): | |||
| list: A list of the volumetric data (length >1 for CHGCAR files with spin) | |||
|
|
|||
| """ | |||
| if os.stat(filename).st_size == 0: | |||
There was a problem hiding this comment.
To have the same behaviour when parsing empty files we should also raise a warning here:
s.logger.warning("File:" + filename + "seems to be corrupted/empty")
| @@ -69,6 +69,8 @@ class with very minor modifications. The new parser is faster | |||
| normalize (boolean): Flag to normalize by the volume of the cell | |||
|
|
|||
| """ | |||
| if os.stat(filename).st_size == 0: | |||
There was a problem hiding this comment.
To have the same behaviour when parsing empty files we should also raise a warning here:
s.logger.warning("File:" + filename + "seems to be corrupted/empty")
pyiron/vasp/outcar.py
Outdated
| line_ngx = lines[trigger_indices[0]-2] | ||
| try: | ||
| line_ngx = lines[trigger_indices[0]-2] | ||
| except IndexError: |
There was a problem hiding this comment.
I do not really like the try except style, because in my feeling this might surpress other errors, therefore I would prefer to check the file size and raise an error directly.
There was a problem hiding this comment.
This fix is unrelated to the CHGCAR parsing fix. This is for calculations using the line mode kpoints (for band structures like Felix does) where theese lines are missing. I think this is an appropriate way to handle this. This pull request handles bugs in the OUTCAR parser as well.
There was a problem hiding this comment.
Can not we simply check the length of the trigger_indices ?
There was a problem hiding this comment.
Yes. But the only possible error that can be raised in this case is an IndexError. In 99% of the cases the if statement will be True. So using the EAFP philosophy a try and except statement would be better
There was a problem hiding this comment.
That is right. Personally I still prefer to have the if statement, as it is just one line of code and I can not see how it helps to clean the code by adding more lines.
There was a problem hiding this comment.
Okay, implemented here
No description provided.