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

Deprecate Python 3.6 & 3.7; get CI working again #270

Merged
merged 16 commits into from
May 25, 2023

Conversation

BrianPugh
Copy link
Contributor

@BrianPugh BrianPugh commented Apr 15, 2023

Change Log

  • Dropped python 3.6 and python 3.7 support.
  • Added python 3.10 and python 3.11 to all the relevant places
  • Updated pre-commit hooks to latest versions
  • Replaced deprecated isort flag -sl with current --force-single-line-imports.
  • Change "extras" dependencies to modern poetry dependency groups.
    • Removed all sphinx-related doc dependencies from the main dependency group.
  • Fixed a few minor typing issues with merge_styles
  • Explicitly set the prompt_toolkit version to be <=3.0.36 because 3.0.37 has introduced some event-loop-related bug.
  • make clean will now also clean the docs.

Original Post

What is the problem that this PR addresses?

  • isort 5.10.1 has the issues described here.

How did you solve it?

  • Ran pre-commit autoupdate to update all plugins; fixing minor linting issues in changes of Black.

Checklist

  • I have read the Contributor's Guide.
  • I will check that all automated PR checks pass before the PR gets reviewed.

@kiancross
Copy link
Collaborator

Thanks for the PR. It looks like the CI is broken for the repo at the moment. I will have to look into fixing this before I can merge!

@BrianPugh
Copy link
Contributor Author

Seems like issues with some CI running on older python 3.7. I'd recommend dropping support (it's at EOL in 2 months anyways)

@kiancross
Copy link
Collaborator

kiancross commented May 13, 2023

@BrianPugh Could you submit a PR to drop support? I cannot approve my own PRs, so would need to wait until another maintainer is available to get it through if I do it...

@BrianPugh
Copy link
Contributor Author

on it 👍

@BrianPugh BrianPugh force-pushed the update-pre-commit-plugins branch 2 times, most recently from 937eed1 to 2f1b5df Compare May 14, 2023 21:28
@kiancross
Copy link
Collaborator

Thanks, much appreciated. Would you be able to submit a separate PR for it?

@BrianPugh
Copy link
Contributor Author

I'm still working on fixing this up, I'll let you know when it's ready. Why the separate PR?

@kiancross
Copy link
Collaborator

I'm still working on fixing this up, I'll let you know when it's ready. Why the separate PR?

It's good practice to have only one change per PR. It also means the commit history has an entry per change. Finally, it means changes are not missed when the changelog is written.

@BrianPugh
Copy link
Contributor Author

yeah, but in this case it's kind of complicated since CI is broken.

  • isort version needs to be changed since older versions are broken with poetry.
  • newer versions of isort that fix this only support >= python3.8.
  • prior to the completion of this PR, there are typing errors, so CI would break there.
  • poetry doesn't support python 3.6

So like this could sort of be broken up into multiple PRs, but it'd be a lot more work for basically no benefit since the goal is to get everything working again with community-supported versions of python.

@kiancross
Copy link
Collaborator

Fair enough. Keep it in this one, and I'll make sure the commit description contains all the changes. Tag me when you're ready for me to review/merge :)

@BrianPugh
Copy link
Contributor Author

@kiancross I think this is getting close to complete. What I've done so far:

  • Updated pre-commit hooks
  • Replaced deprecated isort flag.
  • Updated poetry
    • Updated dependency groups to the modern poetry >=1.2.0 format.
  • Dropped python 3.6 and python 3.7 support.
  • Added python 3.10 and python 3.11 to all the relevant places
  • Fixed a few minor typing issues with merge_styles
  • Explicitly set the prompt_toolkit version to be <=3.0.36 because 3.0.37 has introduced some event-loop-related bug.
  • make clean will now also clean the docs.

What I can't figure out:

  • I've never really used intersphinx before, and I'm getting warnings (that get promoted to errors) like:
/Users/brianpugh/projects/questionary/questionary/question.py:docstring of questionary.question.Question:1: WARNING: py:class reference target not found: prompt_toolkit.application.application.Application
/Users/brianpugh/projects/questionary/questionary/prompts/text.py:docstring of questionary.prompts.text.text:33: WARNING: py:class reference target not found: prompt_toolkit.styles.style.Style
/Users/brianpugh/projects/questionary/questionary/prompts/text.py:docstring of questionary.prompts.text.text:41: WARNING: py:class reference target not found: prompt_toolkit.lexers.base.Lexer
/Users/brianpugh/projects/questionary/questionary/prompts/password.py:docstring of questionary.prompts.password.password:35: WARNING: py:class reference target not found: prompt_toolkit.styles.style.Style
/Users/brianpugh/projects/questionary/questionary/prompts/path.py:docstring of questionary.prompts.path.path:24: WARNING: py:class reference target not found: prompt_toolkit.shortcuts.prompt.CompleteStyle
/Users/brianpugh/projects/questionary/questionary/prompts/path.py:docstring of questionary.prompts.path.path:36: WARNING: py:class reference target not found: prompt_toolkit.completion.base.Completer
/Users/brianpugh/projects/questionary/questionary/prompts/path.py:docstring of questionary.prompts.path.path:39: WARNING: py:class reference target not found: prompt_toolkit.styles.style.Style
/Users/brianpugh/projects/questionary/questionary/prompts/confirm.py:docstring of questionary.prompts.confirm.confirm:28: WARNING: py:class reference target not found: prompt_toolkit.styles.style.Style
/Users/brianpugh/projects/questionary/questionary/prompts/select.py:docstring of questionary.prompts.select.select:47: WARNING: py:class reference target not found: prompt_toolkit.styles.style.Style
/Users/brianpugh/projects/questionary/questionary/prompts/rawselect.py:docstring of questionary.prompts.rawselect.rawselect:39: WARNING: py:class reference target not found: prompt_toolkit.styles.style.Style
/Users/brianpugh/projects/questionary/questionary/prompts/checkbox.py:docstring of questionary.prompts.checkbox.checkbox:51: WARNING: py:class reference target not found: prompt_toolkit.styles.style.Style
/Users/brianpugh/projects/questionary/questionary/prompts/autocomplete.py:docstring of questionary.prompts.autocomplete.autocomplete:34: WARNING: py:class reference target not found: prompt_toolkit.completion.base.Completer
/Users/brianpugh/projects/questionary/questionary/prompts/autocomplete.py:docstring of questionary.prompts.autocomplete.autocomplete:45: WARNING: py:class reference target not found: prompt_toolkit.shortcuts.prompt.CompleteStyle
/Users/brianpugh/projects/questionary/questionary/prompts/autocomplete.py:docstring of questionary.prompts.autocomplete.autocomplete:57: WARNING: py:class reference target not found: prompt_toolkit.styles.style.Style

@kiancross
Copy link
Collaborator

Thanks for all the help. If possible, try to keep the PR as limited in scope as possible (i.e., only what is needed to get CI working again).

I've never really used intersphinx before, and I'm getting warnings (that get promoted to errors) like

Try changing the following URL

"prompt_toolkit": ("https://python-prompt-toolkit.readthedocs.io/en/master/", None),

to

https://python-prompt-toolkit.readthedocs.io/en/3.0.36/

If that doesn't work, I can take a look, but probs won't have time for a couple of weeks.

Another problem we have is that the 3.6 CI runs are marked as required, and I can't overrule this. @tmbo Can you remove all CI requirements from the branch rules for now, and I will provide a list of which ones to add back once all the CI issues are fixed?

@BrianPugh
Copy link
Contributor Author

Thanks for all the help. If possible, try to keep the PR as limited in scope as possible (i.e., only what is needed to get CI working again).

All the changes so far are the minimum necessary to get CI working (outside of just disabling type checking).

Try changing the following URL

That worked! looks like some codecov issues that i should be able to tack in the next day or two.

@BrianPugh
Copy link
Contributor Author

@kiancross so I think this all works, so I'd say approve the workflow to run and we'll see, but I think i tested it about as far as I can from my side.

@kiancross
Copy link
Collaborator

The MacOS breakage might be because Poetry also needs updating (see #266).

@BrianPugh
Copy link
Contributor Author

updated; the cache might need to be cleared for it to take effect, though.

Copy link
Collaborator

@kiancross kiancross left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, this is looking good! Thanks for all your help. I've left some comments/questions below. Can you also update the title of this PR to reflect it's current state and provide a list of changes in the description? I will use this for the commit when merging.

.github/workflows/continuous-integration.yml Outdated Show resolved Hide resolved
.github/workflows/continuous-integration.yml Show resolved Hide resolved
docs/Makefile Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
questionary/styles.py Outdated Show resolved Hide resolved
@kiancross
Copy link
Collaborator

Another problem we have is that the 3.6 CI runs are marked as required, and I can't overrule this.

This is sorted now too, so almost ready to merge!

@BrianPugh
Copy link
Contributor Author

my quick todo list (gotta run for now):

  • Revert the things I said I'd revert.
  • Update PR title/main description to be up to date with all the changes we made.

@BrianPugh BrianPugh changed the title Update pre-commit plugins Deprecate Python 3.6 & 3.7; get CI working again May 24, 2023
@BrianPugh
Copy link
Contributor Author

Finally circled back around on this; @kiancross I believe this is ready for another review!

Copy link
Collaborator

@kiancross kiancross left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All looks good, thanks a lot for all your work!

@CRD716
Copy link

CRD716 commented Oct 23, 2023

Is it possible to support the latest version of prompt_toolkit yet?

@stonebig
Copy link

stonebig commented Nov 4, 2023

maybe it needs prompt-toolkit-3.0.40, not yet released , with this prompt-toolkit/python-prompt-toolkit#1799

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.

4 participants