Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

default estimator type #37

Closed
axelpale opened this issue Oct 28, 2024 · 2 comments
Closed

default estimator type #37

axelpale opened this issue Oct 28, 2024 · 2 comments

Comments

@axelpale
Copy link
Owner

axelpale commented Oct 28, 2024

Current nudged.estimate throws an error if estimator property is not defined.

This could be improved by:

  • select a default estimator type, for example TSR, if the property is not defined.

However, this change can introduce hard-to-detect bugs if the user wrongly remembers or types the property name. For example if the user writes type: 'TS' or estimate: 'TS' instead of estimator: 'TS' then the bug will be found only after an unexpected multi-pointer rotation occurs.

@axelpale
Copy link
Owner Author

Additionally, a missing or bad estimator property currently throws a misleading toUpperCase error.

Likely the best course is to check availability of params.estimator and throw a proper error message if the property is missing.

@axelpale
Copy link
Owner Author

axelpale commented Oct 28, 2024

Additionally, there is a bug in the default switch case of the estimate function, near line 80. The error message should read params.estimator but it reads params.type instead. Replace it with params.estimator.

The bug can be seen in the error message:

> nudged.estimate({
  estimator: 'RRR',
  domain: ...
  range: ...
})
Uncaught Error: Unknown estimator type: undefined

The correct error message should be Uncaught Error: Unknown estimator type: RRR.

axelpale added a commit that referenced this issue Oct 28, 2024
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

No branches or pull requests

1 participant