Updated the path for the jar libraries. Fixes issue #41. #53
Conversation
sutime/sutime.py
Outdated
| @@ -89,8 +89,8 @@ def __init__( | |||
| self._is_loaded = False | |||
| self._sutime = None | |||
| self._lock = threading.Lock() | |||
| module_root = Path(__file__).resolve().parent | |||
| self.jars = Path(jars) if jars else module_root / 'jars' | |||
| self.jars = Path(jars) if jars else os.path.join( | |||
There was a problem hiding this comment.
You're mixing os.path and pathlib.
It should be
Path(jars) if jars else Path(importlib.util.find_spec('sutime').origin).parent / 'jars'
Does Path(__file__).resolve().parent not resolve correctly to the module root for you?
There was a problem hiding this comment.
Hi, I don't have any issue with pathlib. I just find os.path more flexible. I corrected it now.
There was a problem hiding this comment.
Oh, I didn't mean pathlib vs os.path. I'm just wondering whether the original logic didn't work for you and why :/
There was a problem hiding this comment.
Sorry, read too fast... The original code resolves to the working folder, not the environment's library folder. I came accross this issue both on a Mac running Catalina and Ubuntu 18. I am using anaconda as the environment manager on both, so this could be the culprit.
There was a problem hiding this comment.
Hmmm, it should not resolve to the working folder, but rather the module itself (Path(__file__) should resolve to the python file in the module folder). I saw that you closed the PR. Did you find the root cause for your issue?
There was a problem hiding this comment.
Hi, I finally ended up not using this module, because it was much slower than datefinder - for instance. I would prefer it because it provides much more detail, but speed is an issue. The original issue (#41) still remains in my configuration (Mac with Conda). The module only works with the fix I suggested. I see that many people report the same issue, so I think it -at least- needs to be investigated. Let me know whether you want me to reopen the pull request once you have investigated or add the fix yourself. Thanks!
There was a problem hiding this comment.
Great work btw making this API!
|
Code Climate has analyzed commit a9d0812 and detected 0 issues on this pull request. View more on Code Climate. |
@FraBle Please have a look at this fix for issue #41. I think that this is the most appropriate place for the fix, but please share your thoughts.