Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pin Jedi dependency. #12751

Merged
merged 1 commit into from Jan 11, 2021
Merged

Pin Jedi dependency. #12751

merged 1 commit into from Jan 11, 2021

Conversation

yurzo
Copy link

@yurzo yurzo commented Dec 31, 2020

Pins Jedi for the 7.x branch since master its fixed already.
Addresses #12740 and #12745

@NeilGirdhar
Copy link

NeilGirdhar commented Dec 31, 2020

Isn't the master ipython already compatible with the newest jedi?

@yan12125
Copy link

yan12125 commented Jan 1, 2021

Isn't the master ipython already compatible with the newest jedi?

Yep, so the author of this pull request targets 7.x branch instead of master. Compatibility with Jedi 0.18 (dcd9dc9) is not backported to 7.x branch - the latter still uses a parameter removed from the latest Jedi API: https://github.com/ipython/ipython/blob/7.x/IPython/core/completer.py#L1374

@NeilGirdhar
Copy link

Oh, got it, didn't notice the target branch!

@bollwyvl
Copy link
Contributor

bollwyvl commented Jan 8, 2021

@yurzo thank you! I know it's an annoying thing, but could you re-format the lines that just change the single quotes to double quotes? I'm sure we'll get around to blackening the codebase at some point, but until then, it's a lot easier to see changes when a one-line fix is a one-line change.

@Carreau this is starting to cause lots of ripples, and is getting patched in other places... we've pinned on conda-forge, but it can have other consequences. Can you take a look at getting this, or equivalent change in!

@yurzo
Copy link
Author

yurzo commented Jan 8, 2021

@bollwyvl I'd be happy to remove all the quotes.

My original commit was against master, mostly out of ignorance of this project's governance, and it was a minimal Jedi pin (no double quotes).
However, it failed the darker checks. I did not know darker but I'm a fan of black so I excitedly acknowledged darker's suggestions.
This morning, prompted by your request, i diffed master vs 7.x and I estimate that darker is enabled on both, so if I do remove the double quotes, checks will fail.

If per project governance I can make this PR as you ask and it can be merged despite failing checks, please let me know and I'll go push a new patch.

@bollwyvl
Copy link
Contributor

bollwyvl commented Jan 8, 2021

Yeah, I haven't been tracking to what the state of automation is... ✔️ is good!

@Carreau
Copy link
Member

Carreau commented Jan 9, 2021

Sorry I've been a bit overwhelmed. I can't have a look this week end but will try next week.

@bollwyvl
Copy link
Contributor

bollwyvl commented Jan 9, 2021

Understandable, a lot of things are overwhelming right now. Take care of yourself, for sure!

@Carreau Carreau merged commit dff120d into ipython:7.x Jan 11, 2021
@Carreau
Copy link
Member

Carreau commented Jan 11, 2021

Thanks ! I'll try to make a release soon. Will try to get some of the recent PRs in, and do release notes. This week is a bit packed.

@h-vetinari
Copy link

Thanks! :)

May I just ask though - is there a reason why dcd9dc9 shouldn't be backported to 7.x (possibly with a version-guard to keep compatibility with old jedi-versions)? I ask because 8.0 still seems a while off.

@davidhalter
Copy link

It's not a problem - Jedi 0.18.0 fixes only minor issues for IPython. The release gets rid of Python 2, which was its major goal (and which does not matter for IPython).

@h-vetinari
Copy link

It's not a problem - Jedi 0.18.0 fixes only minor issues for IPython. The release gets rid of Python 2, which was its major goal (and which does not matter for IPython).

Respectfully, I disagree. If Jedi 0.18.0 breaks ipython 7.x (the very reason for this PR) and 8.x is not happening in the next ~2 months, then - if possible without crazy complexity - IMO 7.x should stay compatible with the newest jedi.

@SamuelMS
Copy link

SamuelMS commented Jan 27, 2021

Thanks ! I'll try to make a release soon. Will try to get some of the recent PRs in, and do release notes. This week is a bit packed.

@Carreau Just adding my voice to the clamor here – this is a pretty frustrating issue, as all of our dependent tools (Jupyter, ipdb, Django's shell_plus extensions, etc) break both locally and in production environments. Can you please cut a release to 7.x that pins the version? It can be based on 7.9 with no other changes from master – that'd be plenty.

See #12740 for more emphasis.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants