Added suggestion for mistyped/wrong parameters #300
Conversation
Codecov Report
@@ Coverage Diff @@
## master #300 +/- ##
==========================================
+ Coverage 66.55% 67.04% +0.48%
==========================================
Files 59 60 +1
Lines 6196 6308 +112
==========================================
+ Hits 4124 4229 +105
- Misses 2072 2079 +7
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
lumen/base.py
Outdated
| matches = "', '".join(get_close_matches(component_type, cls_types)) | ||
| if matches: | ||
| msg += f" Did you mean: '{matches}'?" | ||
| raise ValueError(msg) |
There was a problem hiding this comment.
Should this code be in Param instead? It doesn't seem specific to Lumen.
There was a problem hiding this comment.
The validate_parameters could/should be implemented in Param as an option instead of "only" emitting a warning. holoviz/param#174
For the code in this function, in particular, I don't think it can be ported to param.
There was a problem hiding this comment.
You don't think that Param could raise a helpful exception when given an attribute name that is not a parameter, guessing the right parameter name the same as done here? I must be missing something, then.
There was a problem hiding this comment.
Yes! A helpful exception for Param would be a very nice addition.
But that is not what this function does, which tries to match the input string component_type to a lumen component and return it.
| @@ -16,6 +18,8 @@ class Component(param.Parameterized): | |||
|
|
|||
| def __init__(self, **params): | |||
| self._refs = params.pop('refs', {}) | |||
| expected = list(self.param.params()) | |||
| validate_parameters(params, expected, type(self).name) | |||
There was a problem hiding this comment.
Reasonable at this level, note though that a lot of components simply collect any parameters that haven't been explicitly declared into self.kwargs so this will never be triggered in those cases.
Co-authored-by: Philipp Rudiger <prudiger@anaconda.com>
|
I have found another config:
title: Lumen
sources:
penguins:
type: file
tables:
penguins: https://raw.githubusercontent.com/rfordatascience/tidytuesday/master/data/2020/2020-07-28/penguins.csv
targets:
- title: Table
source: penguins
views:
table:
type: table
table: penguins
transform: # missing an s here
- type: iloc
end: 100Which does not raise an error, but the following warning: WARNING:param.Tabulator00842: Setting non-parameter attribute transform=[{'type': 'iloc', 'end': 100}] using a mechanism intended only for parameters |
|
I think it could make sense to have a custom error like SpecificationError. Maybe when this error is raised, it would only show the message and not the traceback. |
Co-authored-by: Philipp Rudiger <prudiger@anaconda.com>
Fully agree! |
|
Superseded by #312 (kept all your commits) |
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
It can be hard to know if something is
transformortransforms. I have tried to take account of this by usingdifflibto come up with suggestions if parameters are mistyped/wrong.I have chosen to raise an error when an input parameter does not match the class parameters. My reasoning for this is that the error usually will arrive later, making it harder to debug.
I have used the following example to see if I can provoke an error by adding or removing an
s.The example will raise an error (try to find out what raises it).
Error message
Before:
After: