Skip to content

add boltons - #8892

Merged
AlexWaygood merged 68 commits into
python:mainfrom
AABur:boltons
Jun 7, 2023
Merged

add boltons#8892
AlexWaygood merged 68 commits into
python:mainfrom
AABur:boltons

Conversation

@AABur

Copy link
Copy Markdown
Contributor

add stubs for boltons library

@AABur

Copy link
Copy Markdown
ContributorAuthor

@sobolevn
Please review the bolton/dictutils stubs
This is my first PR in typeshed and there is for sure room for improvement.

@github-actions

This comment has been minimized.

sobolevn
sobolevn previously requested changes Oct 15, 2022

@sobolevnsobolevn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks, this is good start.

I think we would need to:

  1. Fix flake8. Right now it says:
 ./stubs/boltons/boltons/timeutils.pyi:46:20: F821 undefined name 'DSTEND_1987_2006'
./stubs/boltons/boltons/funcutils.pyi:45:5: F811 redefinition of unused 'get_sig_str' from line 43
./stubs/boltons/boltons/funcutils.pyi:46:5: F811 redefinition of unused 'get_invocation_str' from line 44
./stubs/boltons/boltons/funcutils.pyi:54:5: F811 redefinition of unused 'add_arg' from line 53
./stubs/boltons/boltons/urlutils.pyi:92:1: F811 redefinition of unused 'OrderedMultiDict' from line 5
./stubs/boltons/boltons/deprutils.pyi:5:26: F821 undefined name 'ModuleType'
./stubs/boltons/boltons/ioutils.pyi:5:13: F821 undefined name 'unicode'
./stubs/boltons/boltons/tbutils.pyi:3:8: F821 undefined name 'unicode'
./stubs/boltons/boltons/fileutils.pyi:50:12: F821 undefined name 'copy_tree'
./stubs/boltons/boltons/iterutils.pyi:66:5: F811 redefinition of unused '__next__' from line 65
./stubs/boltons/boltons/iterutils.pyi:72:5: F811 redefinition of unused 'reseed' from line 71

You need to define / import all missing names.

  1. Mypy and other tools must pass, there are multiple issues there at the moment. For example,
stubs/boltons/boltons/listutils.pyi:26: note: Superclass:
stubs/boltons/boltons/listutils.pyi:26: note: @overload
stubs/boltons/boltons/listutils.pyi:26: note: def sort(self, *, key: None = ..., reverse: bool = ...) -> None
stubs/boltons/boltons/listutils.pyi:26: note: @overload
stubs/boltons/boltons/listutils.pyi:26: note: def sort(self, *, key: Callable[[Any], Union[SupportsDunderLT[Any], SupportsDunderGT[Any]]], reverse: bool = ...) -> None
stubs/boltons/boltons/listutils.pyi:26: note: Subclass:
stubs/boltons/boltons/listutils.pyi:26: note: def sort(self) -> None

The only way to fix it is to add # type: ignore to the method definition line: def sort(self) -> None: # type: ignore

Comment threadstubs/boltons/boltons/cacheutils.pyi Outdated
Comment threadstubs/boltons/boltons/cacheutils.pyi Outdated
Comment threadstubs/boltons/boltons/cacheutils.pyi Outdated
Comment threadstubs/boltons/boltons/iterutils.pyi Outdated
Comment threadstubs/boltons/boltons/iterutils.pyi Outdated
Comment threadstubs/boltons/boltons/tbutils.pyi Outdated
@github-actions

This comment has been minimized.

@AABur

Copy link
Copy Markdown
ContributorAuthor

flake8 & mypy FIXED

--- TEST SUMMARY: One or more tests failed. See above for details. ---
Flake8: Success
Check consistent: Success
Check new syntax: Success
Pyright: Success
mypy: Success
stubtest: Failed
pytype: Success
Regression test: Success

Time to dive into designing the correct stubs

@github-actions

This comment has been minimized.

@AlexWaygood

Copy link
Copy Markdown
Member

Thanks @AABur. Are you able to take a look at the stubtest errors in CI? Please say if you need guidance on any of them :)

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Comment threadstubs/boltons/@tests/stubtest_allowlist.txt Outdated
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@github-actions

Copy link
Copy Markdown
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@github-actions

Copy link
Copy Markdown
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@AABur
AABur requested a review from AlexWaygoodApril 17, 2023 20:07
@github-actions

Copy link
Copy Markdown
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@github-actions

Copy link
Copy Markdown
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@github-actions

Copy link
Copy Markdown
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@JelleZijlstra

Copy link
Copy Markdown
Member

I went over the code locally and made a number of changes:

  • Target boltons 23.0.0, which has now been released
  • Remove various compatibility aliases (e.g., unicode) that shouldn't be used directly
  • Fix many incorrect uses of NoReturn
  • Fix various other incorrect types
  • Add some missing types

Stubtest will probably have some complaints

@github-actions

Copy link
Copy Markdown
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@JelleZijlstraJelleZijlstra left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There may be some more issues, but I think I caught most of them.

@AlexWaygood do you want to take another look?

@github-actions

Copy link
Copy Markdown
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@AlexWaygood
AlexWaygood dismissed their stale reviewJune 7, 2023 06:52

I trust Jelle

@AlexWaygood

Copy link
Copy Markdown
Member

There may be some more issues, but I think I caught most of them.

@AlexWaygood do you want to take another look?

If you're happy, I'm happy :)

Let's merge this now

@AlexWaygood
AlexWaygood merged commit 46de19d into python:mainJun 7, 2023
@sobolevn

Copy link
Copy Markdown
Member

Congrats @AABur 🎉

@sobolevn

Copy link
Copy Markdown
Member

@kyawgyi99996 what kind of help do you need?

Sign up for freeto 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.

5 participants

@AABur@AlexWaygood@sobolevn@JelleZijlstra@amaslenn