You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The following is a technical comment by one of the JOSS reviewers.
Error handling for 1) .cif files that do not contain specific attributes 2) invalid input file.
Just a few of final minor points:
... (omitted)
... (omitted)
I tried some more random CIF files from CSD/ICSD/MP etc. and all now work without any errors (:confetti_ball:), but some functionality that I expected to work did not. For example, in cases where the CIF only provides chemical_formula_sum and not chemical_formula_structural, the histogram generation for an formula/structure etc. for the ensemble fails. Similarly, when trying to generate polyhedra for these CIFs, the program hangs in cases where invalid input is provided (i.e., not providing a label). You could say these are either user errors or data source errors, but I think for a package aimed at non-specialists it would be helpful to provide a clear error message and fail early in cases where this info is missing. I don't see this as blocking for the paper.
The following is a technical comment by one of the JOSS reviewers.
Error handling for 1) .cif files that do not contain specific attributes 2) invalid input file.
Just a few of final minor points:
chemical_formula_sum
and notchemical_formula_structural
, the histogram generation for an formula/structure etc. for the ensemble fails. Similarly, when trying to generate polyhedra for these CIFs, the program hangs in cases where invalid input is provided (i.e., not providing a label). You could say these are either user errors or data source errors, but I think for a package aimed at non-specialists it would be helpful to provide a clear error message and fail early in cases where this info is missing. I don't see this as blocking for the paper.Originally posted by @ml-evs in openjournals/joss-reviews#7205 (comment)
The text was updated successfully, but these errors were encountered: