Say what a type mismatch expected and what it got - #122
Merged
Conversation
The message was just 'Input 102b: type mismatch for input mean'. It named neither the expected nor the supplied type, and the XML parser error that wraps it points at the enclosing element rather than the offending child, so the reported location is only ever approximate. Working out what was wrong meant reading the source of the class declaring the input. This is worst during migration, because a legacy BEAST 2 class and its BEAST 3 spec twin usually share a simple name and are indistinguishable in the message. In XML a bare element name silently resolves to the legacy class, which is an easy mistake to make and a slow one to find -- it cost us a round trip on BEAST2-Dev/bModelTest#7. Report the input, the object carrying it, the expected type with its domain, and the supplied type; and when the supplied class is a legacy beast.base class where a beast.base.spec one is wanted, say so and show the explicit spec= form. Before: Input 102b: type mismatch for input mean after: Input 102b: type mismatch for input 'mean' of beast.base.spec.inference.distribution.Exponential id='post' expected: beast.base.spec.type.RealScalar with domain beast.base.spec.domain.PositiveReal but got: beast.base.inference.parameter.RealParameter beast.base.inference.parameter.RealParameter is a legacy BEAST 2 class, but this input expects a BEAST 3 spec class. In XML this usually means a bare element name was used, as in <RealParameter name='mean'>, which resolves to the legacy class. Name the input and give it an explicit spec instead, as in <mean spec='beast.base.spec...'>. beast-base suite: 462 tests, 0 failures.
This file contains hidden or 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
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.
Problem
A type mismatch reports only the input name:
It names neither the expected nor the supplied type. The
XMLParserExceptionthat wraps it prints an "Error detected about here" path built from the enclosing element, not the offending child —setInputis handed the parent node and the child never reaches the message — so the reported location is only ever approximate. In practice, finding the cause means opening the source of whichever class declares the input.This is worst during migration. A legacy BEAST 2 class and its BEAST 3 spec twin usually share a simple name, and in XML a bare element name silently resolves to the legacy class:
That is an easy mistake to make and a slow one to find — it cost a round trip on BEAST2-Dev/bModelTest#7, where the same message appeared with no indication that legacy-vs-spec was the issue.
Change
Report the input, the object carrying it, the expected type with its domain, and the supplied type. When the supplied class is a legacy
beast.baseclass where abeast.base.specone is wanted, say so explicitly and show thespec=form.Before:
After:
Scope is deliberately narrow: only the
102bbranch, which carried no type information at all. Error101already reports both types andXMLParserspecial-cases it, so it is left alone to avoid duplicated text.Tests
InputTypeMismatchMessageTestcovers the four cases: the message names input, owner, expected and actual; a legacy class gets the migration hint; a spec-vs-spec mismatch (vector where a scalar is wanted) does not get a misleading hint; and a correctly typed parameter is still accepted.Full
beast-basesuite: 462 tests, 0 failures.Not addressed
The "Error detected about here" location still points at the enclosing element. Fixing that means threading the child
NodethroughNameValuePairintosetInput, which is a larger change — happy to do it separately if wanted.