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

Allow to declare modules/calls as deprecated #10131

Closed
CarliJoy opened this issue Feb 26, 2024 · 4 comments
Closed

Allow to declare modules/calls as deprecated #10131

CarliJoy opened this issue Feb 26, 2024 · 4 comments
Labels
plugin Implementing a known but unsupported plugin rule Implementing or modifying a lint rule

Comments

@CarliJoy
Copy link

CarliJoy commented Feb 26, 2024

Working in larger groups it is sometime hard to enforce a common style.
Therefore we established a Pylint plugin in which we can define some modules as "deprecated" in order to inform all contributors of preferred alternatives.

I.e. instead of os.path use pathlib or instead of requests use httpx or don't use print but use logging.debug instead.
Or don't use multiprocessing.Pool() but concurrent.futures.ProcessPoolExecutor

The Plugin takes either a module like os.path or a function/class call of a module like multiprocessing.Pool().

In case of a module it warns only on the import of the given module.
In case of a call it warn if the given function/class is import i.e. from multiprocessing import Pool or if only import multiprocessing is done, on every multiprocessing.Pool call.

For each module/call one can define a custom string that is shown to the user, i.e. Don't use os.path because pathlib is more flexible or similar.
Also one can define if the given error is only a mere convention or rather an error.

Having a similar possibility in ruff is the last blocker for a migration.

I guess also other teams would benefit from the possibility to mark certain modules/calls as deprecated/not wanted.

Is it thinkable to add a rule, lets say DEP to ruff that allows to declare different modules as deprecated?

For example:

[[tool.ruff.lint.deprecation]]
old = "os.path"
new = "pathlib"
code = "DEPE001"
message = "Please use pathlib for paths"

[[tool.ruff.lint.deprecation]]
old = "print"
code = "DEPC001"
message = "Consider using logging.debug or click.echo instead of print to make the context of the output more clear."

[[tool.ruff.lint.deprecation]]
old = "multiprocessing.Pool()"
new = "concurrent.futures.ProcessPoolExecutor"
code = "DEPE002"
message = "multiprocessing.Pool multiprocessing.Pool is susceptible to deadlocks in the context of OOM kills. Please use concurrent.futures.ProcessPoolExecutor or our custom implementation instead"

[[tool.ruff.lint.deprecation]]
old = "company_megalib"
code = DEPE003"
message = "Don't use company_megalib anymore but look at <URL> to get a possible replacement"

I haven't seen neither array declaration nor custom codes in ruff yet.
Is this even wanted/okay?
Maybe we should only allow a set of fixed codes like:

  • DEPE001: user-defined-deprecation-error
  • DEPC002: user-defined-deprecation-convention

What do you think, could something like this become part of ruff?

@sscherfke
Copy link

sscherfke commented Feb 26, 2024

Maybe the configuration can be simplyfied to sth. like this:

[tool.ruff.deprecated_modules]  # Results in warnings
"old.modname" = "Plz use other.lib"
"os.path" = "Please use pathlib"

[tool.ruff.forbidden_modules]  # Results in errors
requests = "Use must use httpx for web requests"

It's a bit less flexible, but more clear to read and easier to write.

@MichaReiser MichaReiser added rule Implementing or modifying a lint rule plugin Implementing a known but unsupported plugin labels Feb 26, 2024
@Taragolis
Copy link

banned-api (TID251) ?

@sscherfke
Copy link

Here are the docs for it: https://docs.astral.sh/ruff/settings/#lint_flake8-tidy-imports_banned-api

Looks good enough for me. What do you think, @CarliJoy ?

@CarliJoy
Copy link
Author

Sorry for the late reply @sscherfke implemented the banned-api rules and it works as expected.
Thanks for the clean up MichaReiser

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugin Implementing a known but unsupported plugin rule Implementing or modifying a lint rule
Projects
None yet
Development

No branches or pull requests

4 participants