Conversation
calebcase
force-pushed
the
bug/not-quite-a-leaf
branch
from
1944646 to
a8e7a95
Compare
dpath/segments.py
Outdated
| try: | ||
| return leaf(thing) or len(thing) == 0 | ||
| except: | ||
| return false |
There was a problem hiding this comment.
- Typo: this should be
Falsenotfalse. The fact that the tests pass mean this path isn't getting hit. - I recommend catching
TypeErrorinstead of a bareexcept:, as a bareexceptwill also mask exceptions thrown by the interpreter due to syntax errors, block keyboard interrupts, etc.
There was a problem hiding this comment.
Good catch, the linter apparently needs adjusted. It was configured to catch these at one point.
|
@calebcase can you push the fix ? what do we need to get this merged ? I've merged this to my fork if you guys want to use until this get merged, https://github.com/wsantos/dpath-python |
|
This PR is still broken though; see my comments above |
|
So what is the plan to get this done @dargueta @calebcase ? |
|
@calebcase hasn't fixed the bug I found so we can't do anything with it until they're done. |
|
Hi. I can't really use this library without a fix for this issue. If this PR is abandoned should I open another one? Will it get reviewed and merged? |
There have been four already; really it's up to @calebcase to fix the bug in this PR and @akesterson to merge it. @akesterson you may be able to modify this PR yourself since you're the owner. |
|
Looking forward to seeing this fix merged then. In the meantime I'm using @wsantos's fork |
|
Blocked by #136 |
|
@moomoohk I'm going to abandon this, but it might be worth incorporating it in another PR. |
|
Hi everyone. I'm reopening this PR for review and merging. |
bigsablept
left a comment
bigsablept
left a comment
There was a problem hiding this comment.
This looks good to me. Thanks for getting it ready!
|
Changes will be pushed to PyPI soon enough |
6 participants
This relates to:
#119
And incorporates some of the changes from:
#120
#122
#123