bump jedi compatibility: compare to Path-like object #901
Conversation
|
Curse you Python 2 on Windows. |
|
Hello? |
|
Could we please merge this in now since it seems to be passing all tests? I absolutely hate using projects that have pinned dependencies because it invariably leads to the most wretched layers of dependency hell when you inevitably install some other package further down the line that requires the newest version of the same dependency. Please for the love of all the cute and fluffy animals don't pin dependencies, update your code instead. In this case a friendly stranger has kindly already done it for you (wow! so kind!) so please, pretty please (with an entire cherry sundae on top) could you review it and merge it in? Ughhh, sorry about this mini-breakdown. I just needed to get that out of my system. I'm dealing with another dependency clash over at django-storages due to breaking changes in the azure-storage-blobs api as of a new major version (leading to them just pinning the dependency asfdsfadfsaf) that has drained my sanity to the point where I'm just an empty husk. This PR is triggering all sorts of trauma flashbacks. But for real, it would be awesome if this could get merged in. I dream of the day when |
|
@matthewgdv. I completely agree with you. Unfortunately especially the Jedi author as well as the de-factor maintainers of this project (the spyder-ide guys) have a different opinion about pinning. Cf. the past discussions about previous Jedi/PyLS incompatibilities. See also the heated discussion in ipython/ipython#12740. Do you also use IPython? If so, it is currently another blocker for your environment to update Jedi. |
|
And now imagine, that you have to keep thousands of (Python) packages in sync all together as we do it as RPM maintainers in Fedora/Centos/RHEL. Right now, I'd like to update jedi but I cannot until I can update also all packages that depend on it. |
|
@frenzymadness, I don't even have to imagine. I am the openSUSE maintainer for the Jedi package and regularly have to fight off attempts to update the package, because I know it breaks other stuff. I even once tried to make Jedi 0.17.2 compatible with Parso, which passed the tests but was strongly discouraged by the author. |
Sorry but I lost my write permissions on this repo (I don't know why), so I can't merge PRs now. I don't know how much time it could take to get them back (I'd say one month or two, from past experiences).
This should be much less of a problem with the new pip solver.
Well, as this case has proved, as well as the previous one, if we hadn't pinned Jedi this project would be broken and no one would be around to fix it.
It's similar for us maintainers of open source projects: any new version of a critical dependency can break the entire ecosystem quite easily. That's what the new Jedi version did not only for IPython (as @bnavigator mentioned) but the entire Jupyter ecosystem for more than a month (although Spyder depends on IPython, we avoided that bullet because we pin Jedi to 0.17.2). |
|
@ccordoba12 time to fork pyls (as you already proposed IIRC) or switch to Microsoft's pyls for Spyder then? |
I'll contact Palantir's team again to see what happened (perhaps it was simple mistake on their side). If not. we'll fork this project.
That's not feasible for us because that server is built with C#, and doesn't support all the functionality we already have here. |
|
@ccordoba12 If it comes to this, I would be happy to collaborate on the new fork. I believe that reviving the development (merging some PRs, improving performance etc) is of great interest to both Jupyter and Spyder and could lead to a nice collaboration. |
|
Ok, that's good to know. @bnavigator, @frenzymadness, how much of a problem would a fork represent for Linux distros? I basically haven't done it thinking that it'd be a big headache for you guys. |
|
I don't see a big problem for openSUSE here. Either you choose a slightly different name with a different directory in sitelib and make spyder depend on that. Then we have two packages not conflicting. Or you use the same namespace and we argue that the Palantir package is not maintained anymore so we just switch the package over to the fork. |
|
is there any progress here? Just FYI:
|
|
Hey @krassowski, thanks for offering your help. I was waiting to hear from @frenzymadness, but it seems he agrees with @bnavigator. I already created an organization for this: The next step would be to move this repo under We also need to move this project: https://github.com/palantir/python-jsonrpc-server/ which is a core dependency of If you guys agree with this plan, I can start by adding both repos to that org exactly as they are right now. Unfortunately, the Spyder team is pretty swamped at the moment with preparations to release Spyder 5 in April, but we'll be glad to receive PRs for the tasks I mentioned above. We can also take care of maintaining both repos, pushing releases to PyPI and creating packages in conda-forge. Let me know what you think. |
|
LGTM |
|
I am happy to join this organization and send pull requests there. |
|
CC @bollwyvl |
Great! Thanks for your help! I'll reserve some time for this during the next days. I'll let you know when things are ready. |
|
I'll work on the Arch packaging for this over the weekend. Thanks for the effort! |
|
It's nice to see that things are moving. That is how open-source works!
|
|
Hey everyone, the forks are in place: https://github.com/python-ls/python-ls If someone wants to be part of this new organization, please let me know. |
|
It looks like the forks are detached which makes sending PRs from existing forks less convenient. It might be a little bit easier (but by no means required) to have an attached fork for now so that the existing PRs could be easily redirected and only once the codebase diverges to detach the forks. But this is just nice-to-have. Please let me know if you plan to act on this suggestion, or if I should manually port PRs. Edit see: https://stackoverflow.com/questions/36225381/reattach-a-detached-fork-in-github |
Sorry, I wasn't aware of that possibility, but if it makes your life (and everyone's else with unmerged PRs) easier, I certainly can do it. So the idea is to remove the repos I just created and recreate them as forks of the corresponding Palantir ones? |
|
Thinking about it now, forks don't get issues (the issues functionality on GitHub is disabled for forks) so maybe it is fine to keep as is? |
That's the default, but issues can be enabled on a fork easily (one checkbox in Settings). |
|
Indeed! So in that case I think that attached fork will make our life easier initially and then we can detach at a convenient moment (the procedure of de-forking is explained here) |
|
As far as giving the repo its new name (-ls suffix instead of -language-server), I think that can also be done in Settings after forking. |
|
Ok, attached forks should be in place. Please check again and let me know if everything is fine now. |
|
It works fine 🎉. I managed to send a test PR from my previous fork. I also enabled GitHub Actions as those are by default disabled on forks and after/close open of the PR they were triggered so all is set up. I will send some maintenance PRs next weekend. It might be a good idea to ask users who sent PRs to this repo (like this one, or the one dropping Python 2 support) to re-target those PRs against the fork. |
Agreed. Could you take care of that? |
|
New PR: python-lsp/python-lsp-server#2 |
In the hope that this does not turn into the same disaster as last time (#744, #781), here is a small fix to support the newest Jedi with changed API: Jedi now returns Pathlib objects everywhere. To work with that, you need to compare paths not as strings but as Path-like objects.
Fixes #898