139 visualize systems with topology information only #145
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…or presence of *.edr file before parsing attempt successful.
Pull Request Test Coverage Report for Build 13291507229Details
💛 - Coveralls |
ladinesa
reviewed
Feb 6, 2025
atomisticparsers/gromacs/parser.py
Outdated
| try: | ||
| thermo_data = self.energy_parser | ||
| except Exception as e: | ||
| self.logger.warning(f'Error parsing edr file: {e}') |
There was a problem hiding this comment.
log msg should not be formatted string, remove e, add it as exc_info
ladinesa
reviewed
Feb 6, 2025
atomisticparsers/gromacs/parser.py
Outdated
| edr_file = os.path.basename(self.energy_parser.mainfile) | ||
| sec_input_output_files.x_gromacs_inout_file_eneredr = edr_file | ||
| except TypeError: | ||
| logging.warning(f'Error parsing *.edr file, no energy data available.') |
ladinesa
reviewed
Feb 6, 2025
atomisticparsers/gromacs/parser.py
Outdated
| primary_file = self.get_gromacs_file(primary_ext) | ||
| primary_file_nopath = primary_file.rsplit('.', 1)[0] | ||
| primary_file_nopath = primary_file_nopath.rsplit('/')[-1] | ||
| secondary_file = self.get_gromacs_file(secondary_ext) |
There was a problem hiding this comment.
execute only secondary file search if primary file is not found.
ladinesa
reviewed
Feb 6, 2025
atomisticparsers/gromacs/parser.py
Outdated
| if trajectory_file: | ||
| return [trajectory_file] | ||
| except FileNotFoundError: | ||
| logging.warning(f'No coordinates found, no visualization possible.') |
There was a problem hiding this comment.
return [] here so you do not need additional handling
ladinesa
reviewed
Feb 6, 2025
atomisticparsers/gromacs/parser.py
Outdated
| trajectory_file = _get_file_or_fallback('pdb', 'gro') | ||
| if trajectory_file: | ||
| return [trajectory_file] | ||
| except FileNotFoundError: |
There was a problem hiding this comment.
are there other exceptions?
JFRudzinski
reviewed
Feb 7, 2025
atomisticparsers/gromacs/parser.py
Outdated
| @@ -840,7 +841,10 @@ def parse_thermodynamic_data(self): | |||
|
|
|||
| if not thermo_data: | |||
There was a problem hiding this comment.
Isn't this completely redundant? We already checked if the edr files has keys to parse
atomisticparsers/gromacs/parser.py
Outdated
| self.energy_parser.mainfile = edr_file | ||
| # get it from edr file | ||
| if self.energy_parser.keys(): | ||
| thermo_data = self.energy_parser | ||
| else: |
There was a problem hiding this comment.
What if you have an edr file but there are no keys? Then you won't end up in here!
if edr_file:
if keys:
if not thermo_data:
<try getting from input>right?
atomisticparsers/gromacs/parser.py
Outdated
|
|
||
| return trajectory_file | ||
|
|
||
| try: |
There was a problem hiding this comment.
I don't really understand a few things here:
-
Why the
try:, where can it break in error exactly? -
Why separate the trr/xtc vs. pdb/gro cases? Can't you do something like:
def find_trajectory_files(self):
def _get_traj_file(ext):
primary_file = self.get_gromacs_file(primary_ext)
if primary_file is '':
return None
primary_file_nopath = primary_file.rsplit('.', 1)[0] #! Check what happens if there are multiple files found in get_gromacs_file
primary_file_nopath = primary_file_nopath.rsplit('/')[-1]
return primary_file_nopath
traj_file_priority_list = ['trr', 'xtc', 'pdb', 'gro']
for ext in traj_file_priority_list:
traj_file = _get_traj_file(ext)
if traj_file:
return [traj_file]
return []
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added visualization option from a single frame (
*.grofile or*.pdbfile).Parser now ensures an
*.edrfile is present in the upload before attempting to access it, preventing anAttributeError.