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

Add an alias ImportFrom.module and fix other inconsistencies with the ast module #1338

Open
tristanlatr opened this issue Jan 11, 2022 · 6 comments
Labels
Enhancement ✨ Improvement to a component

Comments

@tristanlatr
Copy link
Contributor

tristanlatr commented Jan 11, 2022

I was expecting my current visitors simply based on the ast module to be working seemlessly with astroid trees, but it turns out some attributes names are not matching the ones in the standrd library.

Current behavior

When trying to get node.module on a fresh astroid.ImportFrom:

AttributeError: 'ImportFrom' object has no attribute 'module'

Expected behavior

ImportFrom.module would be an alias to ImportFrom.modname to match standard library names such that less effort is needed to adopt astroid.

Astroid version: 2.9.3

@tristanlatr tristanlatr changed the title Add an alias Module.module Add an alias ImportFrom.module Jan 11, 2022
@DanielNoord
Copy link
Collaborator

Commit that last changed this attribute name is: c3a35e5

I'm not sure why module wasn't chosen.

@DanielNoord DanielNoord added the Enhancement ✨ Improvement to a component label Jan 11, 2022
@tristanlatr tristanlatr changed the title Add an alias ImportFrom.module Add an alias ImportFrom.module and other inconsistencies with the python ast Jan 12, 2022
@tristanlatr
Copy link
Contributor Author

tristanlatr commented Jan 12, 2022

Other inconsistencies:

ClassDef.decorators.nodes -> ClassDef.decorator_list
Attribute.attrname -> Attribute.name
Attribute.expr -> Attribute.value
Name.name -> Name.id
UnaryOp.op:str -> UnaryOp.op:enum members maybe that should extend str to keep compatibility.

@tristanlatr tristanlatr changed the title Add an alias ImportFrom.module and other inconsistencies with the python ast Add an alias ImportFrom.module and fix other inconsistencies with the ast module Jan 12, 2022
@Pierre-Sassoulas
Copy link
Member

I think we should introduce aliases to match what the ast does (without changing the API we have, only adding aliases), what do you think @hippo91 ?

@cdce8p
Copy link
Member

cdce8p commented Jan 31, 2022

I think we should introduce aliases to match what the ast does (without changing the API we have, only adding aliases)

That will get confusing quickly. These inconsistencies are just something that accumulates with time. It isn't too difficult to live with. Compared to aliases. Try understanding code that uses both simultaneously. I would be against it. Like I explained elsewhere astroid isn't designed to be a drop-in replacement for the ast module.

@tristanlatr
Copy link
Contributor Author

I think the readme should be changed, it currently says:

It [astroid] provides a compatible representation which comes from the _ast module.

Maybe also I'm misunderstanding the sentence and the compatibility we're talking about here is about different python versions, it should probably be rephrased if that's the intended meaning.

@DanielNoord
Copy link
Collaborator

I think an update to the readme would be helpful. Would you want to provide a PR for that? I think you probably have the most experience about what astroid does and does not have in common with ast.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement ✨ Improvement to a component
Projects
None yet
Development

No branches or pull requests

4 participants