Fixes #50 - materialize the properties for completion #52
Conversation
|
|
||
| @property | ||
| def is_ordinal(self): | ||
| return self._type == 'ordinal' |
There was a problem hiding this comment.
Not sure I know which types are 'ordinal' vs 'nominal' -- can you add a docstring to these three?
There was a problem hiding this comment.
sure, I'll have to determine how to word it to make it obvious. The best example I can think of is a date series, they define an order, but I'll dig through docs to see how it's explained
|
LGTM |
|
This is just a suggestion, but it reads a little cleaner to me: And it removes the complaint from the back of my head about using an exception as a sentinel. Tests pass -- but we might want a test to cover exactly what this is trying to accomplish. Now we get a KeyError when you pass an invalid key into the dict |
This also fixes a bug where captions and aliases weren't being populated into the multidict correctly due to insufficient testing on my part originally
|
@t8y8 - That's much cleaner, I like that better because it makes it impossible to pass in that value. I'll update with this change and some additional tests. Though it might not happen today due to off-site. |
| except AttributeError: | ||
| if default_value != AttributeError: | ||
| except KeyError: | ||
| if default_value != _no_default_value: |
There was a problem hiding this comment.
Since we're checking for that object you can use "is not" for identity checking, and then it's impossible for any overridden eq to trick it
There was a problem hiding this comment.
oh good point, that's what I get for rushing!
There was a problem hiding this comment.
Also, boo to me for breaking py3. I've now added running the tests under py3 to my pre-commit hook
|
Now that's one classy PR LGTM |
This also fixes a bug where captions and aliases weren't being populated
into the multidict correctly due to insufficient testing on my part
originally