Skip to content
This repository has been archived by the owner on Apr 22, 2021. It is now read-only.

Removes path support #54

Merged
merged 4 commits into from
Sep 12, 2016
Merged

Removes path support #54

merged 4 commits into from
Sep 12, 2016

Conversation

ajm188
Copy link
Contributor

@ajm188 ajm188 commented Sep 3, 2016

Address #46



tests_inputs_directory = os.path.join(os.path.dirname(os.path.dirname(__file__)), "inputs")
method_to_function = undebt.examples.method_to_function
Copy link
Contributor

Choose a reason for hiding this comment

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

can we just import the module?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

how do you mean?

Copy link
Contributor

Choose a reason for hiding this comment

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

from undebt.examples import method_to_function

instead of

import undebt.examples.method_to_function
method_to_function = undebt.examples.method_to_function

is what paiwei is going for here, I think, and if so I agree.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah got it. Yes I wasn't sure if that would work, but __name__ returns the full module name regardless of how it was imported, so this change is fine. Fixing.

@ajm188
Copy link
Contributor Author

ajm188 commented Sep 12, 2016

@paiweilai bump :)

@paiweilai
Copy link
Contributor

lg2m

return '.'.join(name_parts)
def load_module(module):
"""Loads a module from its name."""
return __import__(module, fromlist=[''])
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the more proper implementation of this is to use importlib.import_module.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@asottile advised against this, in favor of not importing the importlib module

@ajm188 ajm188 merged commit 98d0af5 into master Sep 12, 2016
@evhub
Copy link
Contributor

evhub commented Sep 12, 2016

@ajm188 LGTM once you address my issue above.

Edit: Looks like you just merged it, so interpret the above as a minor issue instead?

@ajm188 ajm188 deleted the removes-path-support branch September 13, 2016 02:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants