Skip to content

Refactor code into packages #41

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

Closed
JukkaL opened this issue Jan 22, 2013 · 10 comments
Closed

Refactor code into packages #41

JukkaL opened this issue Jan 22, 2013 · 10 comments

Comments

@JukkaL
Copy link
Collaborator

JukkaL commented Jan 22, 2013

Currently the implementation does not use packages, because the Python translator does not support them yet (#24). Refactor the implementation to use packages.

The first step could be to translate all modules m into modules mypy.m (e.g. mypy.build). Later on, we could use a deeper hierarchy (e.g. mypy.checker.expressions, mypy.transform.functions).

Also rename mtypes to mypy.types (the name mtypes was used to avoid name clash with the types module).

@ashleyh
Copy link
Contributor

ashleyh commented Jan 27, 2013

I've started this on my package-support branch

@ashleyh
Copy link
Contributor

ashleyh commented Jan 27, 2013

Summary of my changes:

  • moved code into mypy package
  • renamed mypy.py to driver.py to avoid conflict with the package name

Issues:

  • It would be nicer if driver.py could be moved into the package and then invoked with python3 -m mypy.driver but this messes with the code that tries to locate the stubs
  • The tests are currently broken
  • There are circular imports, and the only way I could get the generated python work was to use lots of ugly fully qualified names

I've bootstrapped it to the same branch of mypy-py if you want to try it out.

@JukkaL
Copy link
Collaborator Author

JukkaL commented Jan 28, 2013

Circular imports have been a problem earlier as well. Much of the code was ported from Alore, and Alore places few restrictions on circular imports.

Why can't we use things like from mypy import nodes to keep the old look of the code?

Here are a few more potential ways of dealing with circular imports:

  • If they only affect type signatures and casts, we can allow them and erase the related imports automatically,
    since
    type signatures are erased. For overloads, where we can't just erase them, we can generate fully qualified
    names. This feelts hacky, though, and can break clients that expect the imported name to be there.
  • Automatically move circular imports to the bottom of the file, and rename references within
    the file to use fully qualified names. I'm not sure how reliably the transformation can be done, though.
  • Refactor the code to not have circular imports. This probably involves at least moving more code to the nodes
    module. Also type checking and transform related modules may have to be combined (though I guess this is
    less problematic there, and it's easier to live with the fully qualified names).

@ashleyh
Copy link
Contributor

ashleyh commented Jan 28, 2013

I tried from mypy import nodes initially but the essential difference is
the following:

  • from mypy import nodes does the getattr(mypy, 'nodes') immediately
    (doesn't work, since mypy.nodes doesn't exist until the circular import
    has finished)
  • import mypy.nodes defers the getattr until we refer to
    mypy.nodes.X, which in this case is late enough for the import to have
    finished

I suppose the reason this feels ugly is that circular imports are more
common in statically typed code than dynamically typed code (I guess) so
the original python import system design didn't have much pressure to make
circular imports particularly nice.

What if we rewrite from p import m where m is a module to import p.m,
then rewrite references to m later to p.m?

On 28 January 2013 09:44, Jukka Lehtosalo [email protected] wrote:

Circular imports have been a problem earlier as well. Much of the code was
ported from Alore, and Alore places few restrictions on circular imports.

Why can't we use things like from mypy import nodes to keep the old look
of the code?

Here are a few more potential ways of dealing with circular imports:

  • If they only affect type signatures and casts, we can allow them and
    erase the related imports automatically, since type signatures are erased.
    For overloads, where we can't just erase them, we can generate fully
    qualified names. This feelts hacky, though, and can break clients that
    expect the imported name to be there.

  • Automatically move circular imports to the bottom of the file, and
    rename references within the file to use fully qualified names. I'm not
    sure how reliably the transformation can be done, though.

  • Refactor the code to not have circular imports. This probably
    involves at least moving more code to the nodes module. Also type checking
    and transform related modules may have to be combined (though I guess this
    is less problematic there, and it's easier to live with the fully qualified
    names).


    Reply to this email directly or view it on GitHubhttps://github.com/Refactor code into packages #41#issuecomment-12774501.

@JukkaL
Copy link
Collaborator Author

JukkaL commented Jan 28, 2013

Ah, now I see.

It seems that import a.b looks up 'a.b' directly from the sys.modules dict, but from a import b looks up module a, and then the attribute b of a (as you said). So another alternative might be to do something like this instead of from a import b:

import a.b
import sys as __sys
b = __sys.modules['a.b']

Quick testing indicates that this works. However, it adds a to the module name space. So to be safe, we may need to do something even more ugly:

__a = a   # only if a is defined in the module
import a.b
a = __a  # or del a, if a is not defined in the module
import sys as __sys
b = __sys.modules['a.b']

(The above has not been tested, though.)

Not sure which of the transformations is the better approach. In any case, we should probably only do the transformations if there actually are cyclic imports.

@JukkaL
Copy link
Collaborator Author

JukkaL commented Feb 1, 2013

Here are some ideas.

Statically typed code tends to make heavier use of circular imports, but they primarily affect type declarations. However, I think that in this case it's better to refactor the code to use (fewer) circular imports. But this should be done as a separate task after the refactorings have been merged into master. Later on, we may decide to add better support for circular imports in the Python back end as another separate task.

So I suggest we proceed like this:

  • Use fully qualified names and implement the refactoring so that it works with the current implementation.
    Don't worry if the code is a bit ugly.
  • Do some testing in the branch.
  • Merge the changes to master. Make sure that everything works after the merge.
  • Combine the modules which are the worst affected by the circular imports as a separate task (I'll open an issue).
    Clean up the code.

@ashleyh
Copy link
Contributor

ashleyh commented Feb 1, 2013

When you refer to refactoring in the first step, do you mean refactoring
into packages? (As opposed to refactoring the circular imports)

On 1 February 2013 12:34, Jukka Lehtosalo [email protected] wrote:

Here are some ideas.

Statically typed code tends to make heavier use of circular imports, but
they primarily affect type declarations. However, I think that in this case
it's better to refactor the code to use (fewer) circular imports. But this
should be done as a separate task after the refactorings have been merged
into master. Later on, we may decide to add better support for circular
imports in the Python back end as another separate task.

So I suggest we proceed like this:

  • Use fully qualified names and implement the refactoring so that it
    works with the current implementation. Don't worry if the code is a bit
    ugly.

  • Do some testing in the branch.

  • Merge the changes to master. Make sure that everything works after
    the merge.

  • Combine the modules which are the worst affected by the circular
    imports as a separate task (I'll open an issue). Clean up the code.


    Reply to this email directly or view it on GitHubhttps://github.com/Refactor code into packages #41#issuecomment-12992241.

@JukkaL
Copy link
Collaborator Author

JukkaL commented Feb 1, 2013

Yes, I mean only the original refactoring into packages.

@JukkaL
Copy link
Collaborator Author

JukkaL commented Feb 1, 2013

The other refactoring (removing circular imports) is now issue #93.

@JukkaL
Copy link
Collaborator Author

JukkaL commented Feb 4, 2013

I'm going to rename mtypes to types. Then we can close the issue.

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

No branches or pull requests

2 participants