Guesser checks should wrap NoDataError

[IAlibay]

trafficstars

Related to #4748

In the new guesser API, we often do a check for an attribute and then except on an AttributeError. This makes for an ugly capture for users when a NoDataError gets thrown instead.

[lilyminium]

What do you mean by wrap -- do you mean raising a NoDataError if guessing can't happen, or catching it instead of an AttributeError? I skimmed the MDAKits failures and didn't see anything relating to this, although could have missed something due to going fast

[IAlibay]

@lilyminium looking at some of the failures, MDAnalysis raises a NoDataError not an AttributeError, so the traceback tells you it encountered a NoDataError and then it encountered something else, which is confusing to users. By "wrap", I mean that the except should be looking for both error types.

[lilyminium]

Sorry, it's late here, I'm not sure I'm fully understanding you -- if I rephrase, the issue is that the traceback is confusing because the Topology raises a NoDataError and the guesser is catching an AttributeError (and then raising a ValueError)? Are you suggesting just switching the guesser to catch a NoDataError? (and IMO raising a NoDataError too as would also fall within the spirit of the error).

[IAlibay]

@lilyminium I'm saying we should be catching both errors.

Honestly, if this is late on your end then we'll not do the release and wait until these issues are correctly handled.

[lilyminium]

I’m not following your reasoning here — could you please elaborate? The errors for missing attributes should be NoDataErrors. This subclasses AttributeError, which is probably why the previous logic worked. What’s the benefit of catching extra AttributeErrors?On Oct 20, 2024, at 22:50, Irfan Alibay @.***> wrote: @lilyminium I'm saying we should be catching both errors. Honestly, if this is late on your end then we'll not do the release and wait until these issues are correctly handled.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>