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

feat: update deprecations #177

Merged
merged 1 commit into from
Jul 3, 2024
Merged

feat: update deprecations #177

merged 1 commit into from
Jul 3, 2024

Conversation

nstarman
Copy link
Contributor

@nstarman nstarman commented Jun 28, 2024

After #175 and #173 were / will be merged I took a look and noticed some small improvements.
This PR moves Val out of __all__ and applies the https://peps.python.org/pep-0702/ deprecated decorator to make the deprecations visible to static analyzers.

Hm. Maybe I should move the stuff from here to #173 ?
Also, this PR is a good example of why the edit button on files in GH is not a great way to make a PR.

@coveralls
Copy link

coveralls commented Jun 28, 2024

Pull Request Test Coverage Report for Build 9718124559

Details

  • 5 of 5 (100.0%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.001%) to 99.921%

Totals Coverage Status
Change from base Build 9716367696: 0.001%
Covered Lines: 1257
Relevant Lines: 1258

💛 - Coveralls

plum/util.py Outdated Show resolved Hide resolved
plum/parametric.py Outdated Show resolved Hide resolved
Copy link
Member

@wesselb wesselb left a comment

Choose a reason for hiding this comment

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

Ah, I like this. Should this replace the warning.warn calls? I believe the decorator also raises a warning.

@coveralls
Copy link

coveralls commented Jun 29, 2024

Pull Request Test Coverage Report for Build 9725896976

Details

  • 5 of 5 (100.0%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 99.92%

Totals Coverage Status
Change from base Build 9723266109: 0.0%
Covered Lines: 1255
Relevant Lines: 1256

💛 - Coveralls

@nstarman nstarman marked this pull request as ready for review June 29, 2024 17:57
plum/__init__.py Outdated Show resolved Hide resolved
@coveralls
Copy link

coveralls commented Jul 2, 2024

Pull Request Test Coverage Report for Build 9753685972

Details

  • 5 of 5 (100.0%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 99.92%

Totals Coverage Status
Change from base Build 9741547843: 0.0%
Covered Lines: 1255
Relevant Lines: 1256

💛 - Coveralls

Copy link
Collaborator

@PhilipVinc PhilipVinc left a comment

Choose a reason for hiding this comment

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

LGTM. I'm happy to discover about typing_extensions.deprecated.

By the way @wesselb , a thing that always bothered me about plum is that we have a severely polluted namespace and several modules exposed which are unclear how public they are:

In [1]: import plum

In [2]: plum.[/tab/tab]
  deactivate_union_aliases() dispatcher                 get_context()              isinstance()               List                       multihash()
  Dict                       Function                   get_overloads()            issubclass()               method                     NotFoundLookupError
< dispatch                   function                   is_faithful()              Kind                       Missing                    overload()                 >
  Dispatcher                 get_class()                is_in_class()              kind()                     ModuleType                 parametric()

I think one thing that might be helpful down the line would be to move everything to a _src module, and expose just few things in the main namespace...

@wesselb
Copy link
Member

wesselb commented Jul 3, 2024

@PhilipVinc Hmmm, you’re totally right. It would be considerable improvement to better structure what’s exported where. I agree that the namespace currently is polluted. I’ve noted this down! Thanks! :)

@nstarman, thanks for yet another super clean PR!! I really do appreciate the many small, very well structured PRs. You’ve contributed a lot of improvements lately. :) This also looks great, so merging right away.

@wesselb wesselb merged commit 2528148 into beartype:master Jul 3, 2024
12 checks passed
@nstarman nstarman deleted the patch-1 branch July 3, 2024 20:43
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