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

Importation classes with imported name and alias #54

Merged
merged 1 commit into from
Mar 15, 2016

Conversation

jayvdb
Copy link
Member

@jayvdb jayvdb commented Nov 26, 2015

In order to solve many corner cases related to imports,
more information is needed about each import.

This change creates two new classes:

  • SubmoduleImportation
  • ImportationFrom

And adds an optional parameter full_name to the super class
Importation.

Functionally, this change only improves existing error messages
to report the full imported name where previously an error
would include only the import alias.

@jayvdb
Copy link
Member Author

jayvdb commented Nov 26, 2015

source_statement exists as I would like to include that in a better __repr__

Note that I have a functionally equivalent changeset which does all the work within the existing Importation class: jayvdb@e250ea6 . IMO that isnt as neat and tidy; the special case for _submodule_import adds complexity throughout.

@jayvdb
Copy link
Member Author

jayvdb commented Mar 11, 2016

@bitglue , could you please review this. I have pending changes that depend on this.

@bitglue
Copy link
Member

bitglue commented Mar 11, 2016

Looks good on a brief read. I'll take a deeper look tonight.

@bitglue
Copy link
Member

bitglue commented Mar 14, 2016

This looks good. Can you please rebase onto master?

# A dot should only appear in the name when it is a submodule import
# without an 'as' clause, which is a special type of import where the
# root module is implicitly imported, and the submodules are also
# imported by Python does not restrict which attributes of the root
Copy link
Member Author

Choose a reason for hiding this comment

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

ah, just noticed : 'by' should be 'but' or 'however'

In order to solve many corner cases related to imports,
more information is needed about each import.

This change creates two new classes:
- SubmoduleImportation
- ImportationFrom

And adds an optional parameter full_name to the super class
Importation.

Functionally, this change only improves existing error messages
to report the full imported name where previously an error
would include only the import alias.
def __str__(self):
"""Return import full name with alias."""
if self._has_alias():
return self.fullName + ' as ' + self.name
Copy link
Member Author

Choose a reason for hiding this comment

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

would we prefer %s syntax here instead of + ? It does seem that %s syntax is preferred.

Copy link
Member

Choose a reason for hiding this comment

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

I don't have any particular preference.

@bitglue bitglue merged commit aec68a7 into PyCQA:master Mar 15, 2016
@collinanderson
Copy link

This appears to break relative imports like: from . import urls

@jayvdb
Copy link
Member Author

jayvdb commented May 5, 2016

Yea, there were no tests covering them, and I forgot about those. The fix #61 is waiting for review.

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.

3 participants