bugfix in AutoPopulate.progress()

[dimitri-yatsenko]

No description provided.

[fabiansinz]

I don't understand why we need this. The problem in progress must be the call to populated_from which calls the parents property. I had the same bug, but I introduced a decorator in the dependencies class that should fix this. Have you tried without loading the dependencies in progress after loading my changes?

[dimitri-yatsenko]

Every call of Dependencies.load(), issues a SHOW TABLES query, so it needs to be done sparingly at the start of operations that require dependencies: erd, populate, progress. I will review this.

[fabiansinz]

Then I think it would be best to have a switch in dependencies that indicates whether they were loaded already and a reload=True switch in load that forces a reload. I don't think progress is the right place. Who knows who will call dependencies from outside?

[dimitri-yatsenko]

@fabiansinz What's the advantage of using the decorator @load_dependencies as opposed to explicitly calling self.load() at the start of each method?

[fabiansinz]

I find that more explicit. Since we don't load the dependencies once the object is created, but some function do require loaded dependencies at the same time, I think it is cleaner to mark those functions that need loaded dependencies with that decorator.

I guess it's a matter of style. I like using decorators to make sure some conditions are satisfied and then just use that conditions inside the function instead of ensuring it in the function itself which might serve an entirely different purpose. Argument checking is a classic example. If you put it explicitly in a function, the lines for argument checking can completely clutter the code in which the actual computations of the function happen.

[eywalker]

I'm going to go ahead and accept the pull request, but we still have to decide on a good strategy to load the dependency when needed. @dimitri-yatsenko removed call to load by getting rid of @load_dependencies, but without introducing explicit calls to load at the start of each method. I'm going to create a new issue to track our progress on this matter.