Skip to content

Conversation

@nhuurre
Copy link
Collaborator

@nhuurre nhuurre commented Feb 12, 2024

Submission Checklist

  • Run unit tests
  • Documentation
    • If a user-facing facing change was made, the documentation PR is here:
    • OR, no user-facing changes were made

Since #1024, unknown identifier related errors give a similar identifier as a suggestion but the algorithm only considered typos involving at most three letters. Another likely error is omitting the distribution suffix from a function name:
https://discourse.mc-stan.org/t/how-to-interpret-a-returning-function-was-expected-but-an-undeclared-identifier-was-supplied/34078

Release notes

When encountering an unknown identifier that matches a known suffixed function without its suffix, add the known function name to the error message.

Copyright and Licensing

Copyright holder: Niko Huurre

By submitting this pull request, the copyright holder is agreeing to
license the submitted work under the BSD 3-clause license (https://opensource.org/licenses/BSD-3-Clause)

@codecov
Copy link

codecov bot commented Feb 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (35c682b) 89.88% compared to head (ddacb1a) 89.88%.
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1401   +/-   ##
=======================================
  Coverage   89.88%   89.88%           
=======================================
  Files          63       63           
  Lines       10508    10513    +5     
=======================================
+ Hits         9445     9450    +5     
  Misses       1063     1063           
Files Coverage Δ
src/frontend/Environment.ml 69.49% <100.00%> (+2.82%) ⬆️

@WardBrian
Copy link
Member

This is going to end up being based on the order of the lists of suffixes in Utils, right? E.g. if a lpdf is available, it will never suggest lccdf

That’s already an improvement so I’m happy to merge as is, but how would you feel about giving a list of suggestions?

@nhuurre
Copy link
Collaborator Author

nhuurre commented Feb 12, 2024

If any of them matches then almost all of them are going to match, and I think that's excessive. One option would be to omit _*cdfs and choose either _lpdf/_lpmf or _rng based on the argument types but that adds some complexity.

@WardBrian WardBrian merged commit fa03ea2 into stan-dev:master Feb 12, 2024
@nhuurre nhuurre deleted the suggest-fn-suffix branch February 12, 2024 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants