Erotemic / mkinit

Auto-generate explicit readable __init__.py files without `import *`
Apache License 2.0
73 stars 6 forks source link

`__all__` generation? (and other things) #1

Open vltr opened 6 years ago

vltr commented 6 years ago

Hi! First of all, thanks for this nice tool! I think this tool is something rarely perceived but a very common task for Python programmers and it is super welcome!

Anyway, not that this is an issue, I assume that there are plans to implement this (for linter happiness), but I could also give some thoughts about the __all__ variable because not every single variable from a certain module is needed in the __init__.py file, like an import from another module. So, I can define what I want inside __all__ and that's what will shows up in the __init__.py file for that module, right?

Well, I want to share some situations ... I'm not here to say "hey, this tool is worthless because it doesn't check this or that", but rather give my use case so you can evolve it in the right direction :wink: I really appreciate your effort!

  1. Let's say I have a big file with a lot of classes (like a models.py file), and I don't want to export anything but my model classes, so I'll do something like this:
    
    _all_locals = []

for k in dir(): if k.startswith("_"): continue if k in locals() and inspect.isclass(locals().get(k)): if issubclass(locals().get(k), BaseModel): _all_locals.append(k)

all = tuple(_all_locals)

Unfortunately, this doesn't work with _static_ `mkinit` generation ... Mostly because of the AST, I presume? Given the module root, the AST shouldn't be able to retrive the `__all__` variable result? In this case, it's a plain module.

2. Filters: it is basically related to the above question. What if, inside `__init__.py` we could define some filters to what should or shouldn't belong to the `__init__.py` file? Example:
```python
AUTOGEN_INIT_EXCLUDE = "lambda module, member: not member.startswith('_')"
AUTOGEN_INIT_INCLUDE = "..."
  1. Module level imports: I think this should be optional, I don't find very useful to have a __init__.py file like this (see comments in code):

    # <AUTOGEN_INIT>
    from my.app.models import core  # i don't need this
    from my.app.models import distributed  # nor this
    from my.app.models.core import (Base, BaseModel, metadata,)
    from my.app.models.distributed import (Address, Contact, Device,)
    # </AUTOGEN_INIT>
  2. Another nice option / feature would be to generate relative imports instead of the full module path, example:

    AUTOGEN_INIT_RELATIVE_IMPORT = True
    # <AUTOGEN_INIT>
    from .core import (Base, BaseModel, metadata,)
    from .distributed import (Address, Contact, Device,)
    # </AUTOGEN_INIT>
  3. For last (but not least important), to make linters and / or code style checkers happy, I think mkinit could (optionally) create the __all__ variable at the end of the file / AUTOGEN_INIT block ... Assuming the above example:

    
    AUTOGEN_INIT_RELATIVE_IMPORT = True
    AUTOGEN_INIT_ALL = True
    # <AUTOGEN_INIT>
    from .core import (Base, BaseModel, metadata,)
    from .distributed import (Address, Contact, Device,)

all = ( "Base", # from .core "BaseModel", # from .core "metadata", # from .core "Address", # from .distributed "Contact", # from .distributed "Device", # from .distributed )



Well, those are all ideas and possible proposals that would significantly boost productivity, at least in my use case. Again, this issue is just to give an use case and opinion on how `mkinit` could be a really awesome tool (to me). I'm not here to give destructive thoughts, since I already think this is a great tool and can evolve to something really, really slick in the future! :smile:

My best regards and thanks for your work!!
Richard.
Erotemic commented 6 years ago

Hey @vltr, thanks for giving my tool a try! It's a huge motivator to see this kind of constructive feedback. Lets go point by point.

  1. Correct, your case won't work with static generation. There is no way to generally handle such cases using static analysis without simulating the code, and in that case why bother with static analysis? However, note that mkinit does have a dynamic backend, although it isn't fully hooked up yet. However, it wouldn't be out of the question to put a regular expression attribute name matcher that could be passed to the command line tool. Perhaps there could even be a __mkinit_export__ directive to explicitly state what should be exported. I'm planning on hooking up / documenting how to use the dynamic backend, but I'm not sure when I'll be able to get to it.

  2. I think the idea is good, but I'm not in love with the using comments to define code that will be executed as a directive. I think defining a regex and using a pattern matching solution might do the trick. It is going to take some thought to find the right implementation.

  3. Yes, there does need to be a mechanism for this preference. This should actually be pretty easy to do in the current architecture. If you want to try tackling this in a PR let me know, otherwise I'll get to it once I have some time.

  4. This feature is definitely on my want list. This should be another easy one implementation wise. Probably just a few line changes and passing down a flag variable. Again, PRs are welcome, but I do want this feature in a 0.1.0 release.

  5. Honestly, I didn't know that you could silence the linters by having an __all__ variable in your init file. It makes me happy to know I'll never need to write another # flake8: noqa again. It should be simple to add this feature by modifying mkinit.static_mkinit._initstr.

vltr commented 6 years ago

I'm glad to help, @Erotemic !

Just like black, I think this tool can make a lot of sense in Python development. I'll go by each point as you did:

  1. I mentioned my "alternative" that could not be handled statically because it is something being done not only by me, you can also track some of this "tactics" on way bigger projects. I was just trying to understand the reach of your tool. Perhaps generating the __all__ variable of each file could be an extra feature of this tool? I mean, __all__ variables directly influence the result of the generated __init__.py file.
  2. I agree with you that running code from variables or inside comments may not be the greatest way, but you can keep in mind that this is something that sometimes is used as default behavior in Python itself :smile:
  3. I would love to make a PR, but for now I have no time to even write the docs of one of the projects I help with, imagine new contributions ... But in two or three weeks I think I'll have some spare time, let's see!
  4. Same as above :grimacing:
  5. Ha! I'm happy I could help a fellow Python developer to solve one of the boring tasks of the language! :wink:

Anyway, one thing that came to mind now is that using AUTOGEN_INIT_ variables could raise some linter noises, which leads us to use # noqa on them ... Or use them inside comments, just like most linters works (including # noqa). I think it's just a case of finding a balance :muscle:

Erotemic commented 6 years ago

I wasn't aware of black (I usually use autopep8). I'll check it out.

  1. I would like to eventually make it so you can ask mkinit to run dynamically when running it from the command line. Currently there exists a way to run dynamically from within python, but to autogenerate anything you need to use a command line flag (when running your program), which isn't very clean. Now that I've separated out the logic for output formatting in the static version, the next step for this point is to hook it into the dynamic version.

  2. Its funny that you link to timeit. I made my Timerit tool as an alternative exactly because timeit requires code represented as strings to run. I've been planning on separating it from ubelt and making it a standalone module from some time now, but writing this comment gave me the motivation to actually do it. :smile:

  3. Understood, I think I actually handled points 3, 4, and 5, in #3

My earlier point was not about # NOQA on a single line, I'm ok with that. I'm much less ok with # flake8: noqa because it suppresses linting on an entire file.

For the remaining point 1 and 2, I'm going to make new issues. I'll leave this issue open as a milestone until the all points are satisfied.

vltr commented 6 years ago

Sorry for the delay! I was using autopep8 as well, but black provides such concise code formatting that's almost a pity not to use it. Of course, it's still on beta and you have to add one or two warnings into your flake8 ignore list, but for now that's okay. You can include it in your Makefile or git hook, well, it's an awesome tool. Just a tip: use the --safe flag :smile:

  1. I think this could end up as a nice set of tools. I'm just wondering here this tool integrated with vscode (what I use today to code) and / or other IDEs / editors ... It will be simply awesome!
  2. Hahaha I'm glad to help! I think I'll even use it to rewrite one of my gists, which will serve as parameter for a project I have in mind and in need to implement;
  3. Yeah, I saw #3 yesterday! Great job, thanks a lot! :wink:

I understood what you told about # flake8: noqa ... I was just figuring out if using variables inside __init__.py like AUTOGEN_INIT_ALL = True # noqa was a good idea (I mean, it was my suggestion (quite vague actually), so I just wanted to give something else to think about). For now, this looks really good! Thanks for your time, @Erotemic !

Erotemic commented 5 years ago

@vltr,

The newest 0.2.0 release begins to handle points 1 and 2. You can now specify variables in the __init__.py that modify the behavior of autogeneration.

While this doesn't implement pattern matching, you can use the __explicit__, __private__, and __protected__ variables to define lists of variable / module names. This is what they do:

If you think pattern matching is absolutely necessary, it may be reasonable to allow glob-style strings as items in these lists. I think if that feature lands, then all 5 points will be checked off correct?

Note, that the first feature can be handled by specifying an __all__ variable in the module where you only want to export very specific things. By default mkinit will respect a submodule's __all__ variable.

vltr commented 5 years ago

Hello @Erotemic ! Thanks a lot for the heads up, I had no time to add mkinit to any script or automatic process (yet), but I'm working on some tools that may lead me to do so (I hope).

If you think pattern matching is absolutely necessary, it may be reasonable to allow glob-style strings as items in these lists. I think if that feature lands, then all 5 points will be checked off correct?

Well, yes. I really don't mind (myself) about pattern matching, but I've seen countless Python source codes to know that this is in fact a requirement, but I think a glob matching would do almost all the job. Example glob matching patterns I have found in the wild (but I don't recall now where (exactly)):

Other cases would require a more complex pattern matching, like any stdlib import, some internal flags (like DEBUG, even though it can be part of __private__) or any "full underscore" variable names, such as py3 or py2, that sometimes are used in the source code specially when you're dealing with the presence of one lib and, if not present, fallback to another, like this snippet from the Sanic framework :wink:

But, as always, good work on mkinit, it is really an exceptional tool for any Python utility tool belt IMHO :muscle: