Move KernelBuilder into its own file

[major]

The __init__.py file is becoming quite cluttered and this change makes it easier to test and maintain the KernelBuilder class.

[coveralls]

Pull Request Test Coverage Report for Build 534

• 187 of 189 (98.94%) changed or added relevant lines in 3 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+1.9%) to 51.416%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
skt/executable.py	1	2	50.0%
skt/kernelbuilder.py	100	101	99.01%

Totals
Change from base Build 529:	1.9%
Covered Lines:	817
Relevant Lines:	1589

---

💛 - Coveralls

[spbnick]

Yay, cleanup and organizing :)!

One thing: I've seen this happen often in Python, but it still irks me: do we have to repeat ourselves in the module and in the class name? Can we have sktm.kernel.Builder instead? And perhaps (later) sktm.kernel.Tree too?

[veruu]

One thing: I've seen this happen often in Python, but it still irks me: do we have to repeat ourselves in the module and in the class name? Can we have sktm.kernel.Builder instead? And perhaps (later) sktm.kernel.Tree too?

This is exactly the thing I mentioned to you with the Jenkins class name which doesn't mention Jenkins at all - module information is often only on the import line and lost in the rest of the code, so the class name itself should be descriptive enough to know where it comes from. One of Python's rule is readability, so noone will write a full module path in the code unless they are, for some reason, forced to.

I guess it doesn't matter too much if we are the only ones using the code and there is only one way it can be interpreted or used, but for things like libraries it's more or less required and it's a good practice in general.

[spbnick]

@veruu, I agree having "Builder" or "Tree" imported directly wouldn't work for the majority of modules. With the exception of perhaps modules also dealing with kernel. I agree that this is especially important for libraries.

However I think we should use Python's import system, and avoid repetition. I.e. write skt.kernel.Builder, and e.g. import Builder directly into modules also dealing with the kernel (where KernelBuilder would be redundant), import kernel.Builder into other skt modules (where skt.kernel.Builder might be a bit redundant), and let outside code import the fully-qualified skt.kernel.Builder, or shorten it if they feel it's OK with them.

In short: let the users decide how they want it named and how much qualified the names should be, because Python lets them do that. Let's not impose particular qualification onto the final (leaf) name. We don't know what scope it will have. In the end, KernelBuilder might be just perfect, too long, or too short, depending on the context.

However, if you don't subscribe to my arguments, I'll be OK with skt.kernelbuilder.KernelBuilder too :)

Sorry, keep mixing up sktm and skt :)

[veruu]

Seeing skt.kernel.Builder only reminds me of Zen on Python's "Flat is better than nested" :) Python is not Java, usually you only import the classes you want and don't type the full path anywhere (exactly as @major did with from skt.kernelbuilder import KernelBuilder). It's not about "letting users decide", it's about following common practices (which the users should follow too) and having easily readable code (without parts that don't add anything useful). If that makes sense :)

[spbnick]

Well, I'm not at all a Java fan myself, but I think such practices retract from Python's module system and composition flexibility, and disagree with them, at least ATM. However, it doesn't have anything to do with how we run our project :) Let's just be consistent, so if we go with skt.kernelbuilder.KernelBuilder or skt.kernel.KernelBuilder here, then I'll switch to sktm.jenkins.JenkinsProject in cki-project/sktm#52.

[major]

Thanks for the discussion here (I learned some things!). Is there anything that needs to be changed for this PR to merge?

[spbnick]

@major, TBH, I didn't review your PR thoroughly, as I assume it's just moving things around, no other changes (correct me if I'm wrong). If you and @veruu agree on the location/naming of the kernel builder as it is, I'm alright with merging it, otherwise rename it and then let's merge :)

[major]

@spbnick No other changes are in here except the addition of tests.

I'm with @veruu on the naming. Seeing KernelBuilder in the code tells me right there that I'm going to be building kernels. If we do skt.kernel.Builder, then you would see Builder when the class is used, and that's not as specific.

[spbnick]

My idea was to, yes, have it as sktm.kernel.Builder, but decide how you wanna see it on import. So if you want to have kernel mentioned just import and use it like this:

import sktm.kernel as kernel

builder = kernel.Builder()

So, basically you would have the same "KernelBuilder" just spelled differently, while also allowing calling it just a "Builder", and not repeating ourselves in the hierarchy.

I see, however, that I'm not getting traction with this idea, so let's merge it as is :)

[veruu]

We should probably merge this as it's approved already