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

Support aliases in rope_autoimport #712

Closed
tkrabel-db opened this issue Oct 18, 2023 · 3 comments · Fixed by #730
Closed

Support aliases in rope_autoimport #712

tkrabel-db opened this issue Oct 18, 2023 · 3 comments · Fixed by #730

Comments

@tkrabel-db
Copy link

Is your feature request related to a problem? Please describe.
Context: Using rope_autoimport as a language server feature (like in python-lsp-server).
I commonly type aliases like pd or np, but rope autoimport doesn't suggest the right aliased imports for that.

Describe the solution you'd like
Support autoimports for common aliases. This doesn't have to be comprehensive, but we could start with the obvious ones:

import numpy as np
import pandas as pd
import matplotlib.pyplot as plt
import seaborn as sns
import statsmodels as sm
import sklearn as sk
import tensorflow as tf

Describe alternatives you've considered
I considered solving this in python-lsp-server, which uses rope for its autoimport feature. My conceptual implementation feels hacky and inefficient. Here's why, given an example.

Let's say I want to support one alias import, import pandas as pd. In order to do that on the LS level, I need to search the index to check if pandas is installed, which adds a scan to the scan necessary to find potential import candidates. The index search increases with the number of aliases! So with, say 5 supported aliases, we search the index 5 times for every "aliased" module + 1 more time for the regular import suggestions.

Moving this to rope would reduce the complexity. We could add a get_alias_modules() (name TBD) method that searches a specific alias index in rope (which could be an in-memory object since that index is super small), and add that search to the full_search.

I am happy to implement that if you like the idea and we get consensus on the implementation

@lieryan
Copy link
Member

lieryan commented Oct 23, 2023

Hi @tkrabel-db, thanks for your interest in contributing to rope.

I think the idea sounds reasonable to me. You're welcome to try and implement it, if the code looks reasonable enough, I am happy to review and accept the pull request.

I don't think this should be implemented as a hardcoded list of aliases though, it would be much more sensible to make this a configurable option.

Rope reads project tooling configuration from pyproject.toml or a user-global configuration file pytool.toml (refer to https://github.com/bagel897/pytoolconfig/blob/main/src/pytoolconfig/sources/pytool.py for the typical location for each OS). One of this option could be a dictionary with a list of these aliases for the user.

Let me know if you would need any help.

@tkrabel-db
Copy link
Author

That makes sense! I'll work on it!

@MrBago
Copy link

MrBago commented Nov 7, 2023

@tkrabel-db @lieryan I made an initial pass at fixing this issue in #730, if the approach is reasonable I can keep working on the PR to complete the feature.

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 a pull request may close this issue.

3 participants