Switch guessers to catching and raising NoDataErrors #4755
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #4755 +/- ##
===========================================
- Coverage 93.65% 93.62% -0.03%
===========================================
Files 175 187 +12
Lines 21564 22631 +1067
Branches 3023 3023
===========================================
+ Hits 20195 21189 +994
- Misses 925 998 +73
Partials 444 444 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
From reading the issue and glancing at the code, changing the raised ValueError to NoDataError makes sense and seems more consistent.
I am not really familiar with the new guesser code so this is just a surface-level review. Other reviews would be great.
@lilyminium I'll leave it to you to gather more opinions if you need them.
|
I think what @IAlibay means is that there is a nested exception error message: a = np.array([
[0., 0., 150.], [0., 0., 150.], [200., 0., 150.],
[0., 0., 150.], [100., 100., 150.], [200., 100., 150.],
[0., 200., 150.], [100., 200., 150.], [200., 200., 150.]
])
u = mda.Universe(a, n_atoms=9, to_guess=['elements'])
> ...
> NoDataError: This Universe does not contain name information
> During handling of the above exception, another exception occurred:
> ...
> NoDataError: there is no reference attributes in this universeto guess types fromwhich is kinda ugly. you can explicitly suppress the chaining of exceptions using the from None syntax (see suggestions) |
| except ValueError: | ||
| raise ValueError( | ||
| except NoDataError: | ||
| raise NoDataError( | ||
| "there is no reference attributes" | ||
| " (elements, types, or names)" | ||
| " in this universe to guess mass from") |
There was a problem hiding this comment.
| " in this universe to guess mass from") | |
| " in this universe to guess mass from") from None |
| except AttributeError: | ||
| raise ValueError( | ||
| except NoDataError: | ||
| raise NoDataError( | ||
| "there is no reference attributes in this universe" | ||
| "to guess types from") |
There was a problem hiding this comment.
| "to guess types from") | |
| "to guess types from") from None |
Yes, thanks so much @yuxuanzhuang, that's exactly what I was looking at! Sorry I conflated that with the error type for some reason. |
Fixes #4749 (maybe?)
Changes made in this Pull Request:
PR Checklist
Developers certificate of origin
📚 Documentation preview 📚: https://mdanalysis--4755.org.readthedocs.build/en/4755/