danijar / handout

Turn Python scripts into handouts with Markdown and figures
Apache License 2.0
2.02k stars 106 forks source link

inspect.stack() role in code #21

Closed epogrebnyak closed 5 years ago

epogrebnyak commented 5 years ago

What is the purpose of look-through in Handout class:

for info in inspect.stack():
      if info.filename == __file__:
        continue
      break

My first impression is that nothing happens in the loop.

danijar commented 5 years ago

Feel free to add a comment to the code:

Find the file that created the Handout object, to include its source code in the output document. The loop walks through the call stack, skips entries that are in handout.py which are internal, and breaks at the first external Python file.

epogrebnyak commented 5 years ago

@danijar - it is more clear from the comment, but is there a possiblity to move dealing with source file into own class?

Handout class is busy constructing the html - I think there can be a separate class that provides access to source file:

class Source:
    def __init__(self, info: inspect.FrameInfo):  
        self._info = info

    def filename(self) -> str:
       """Return path to a file where Handout object was created.""" 
       return self._info.filename

    def text(self) -> str:
        """Return contents of a file where Handout object was created."""
        module = inspect.getmodule(self._info.frame)
        return inspect.getsource(module)   

    def current_line(self) -> int:
        """Return current line in a file where Handout object was created."""
        for info in inspect.stack():
          if info.filename == self.filename():
            return info.lineno
        message = (
            "Handout object was created in '{}' and accessed in '{}'. The file in "
            "which you create the handout will be rendered. Thus, it only makes "
            "sense to add to the handout from this file or functions called from "
            "this file. You should not pass the handout object to a parent file.")
        raise RuntimeError(message.format(self.filename(), info.filename))

class Handout:
    def __init__(self, ...):
        # ... 
        # The loop walks through the call stack, skips 
        # internal entries in handout.py, and breaks at 
        # the first external Python file. We assume this 
        # is the file is where a Handout instance was created.
        for info in inspect.stack():
            if info.filename == __file__:
                continue
            break
        self.source = Source(info)

    #later:
        self.source.text()
        self.source.current_line()

As a benefit I see:

danijar commented 5 years ago

I'm not sure whether this restructuring is desirable. I think it would make more sense to move everything HTML related into a separate HTMLBackend and also contains the Block classes as separate functions. That would open the door to add a LaTeXBackend in the future. That being said, I'd say it's a bit early to do this refactoring since we don't know if it will be necessary.

epogrebnyak commented 5 years ago

Handout minus HTMLBackend probably equals Source or similar, imo, but sure differentiating backends is a bigger task than style preferences for Handout alone.

danijar commented 5 years ago

Exactly, that's what I was thinking. On a philosophical level, both could be separated out to separate different sources and targets. But for now I don't see why this would be necessary and I like to defer refactorings until they become necessary, because the "optimal solution" may change until then.

epogrebnyak commented 5 years ago

Right, refactoring options may be wider than we think now when new concerns emerge. Lets try tackle making a pdf and latex outputs in #18 and #9.