Move code from __init__.py to slither.py

[CosmicWebServices]

See #68

Not tested.

[skistaddy]

+1

[CosmicWebServices]

Thanks!

[skistaddy]

:D

[BookOwl]

I don't like this, it means that instead of just importing slither, users have to do from slither import slither, which just seems weird.

[CosmicWebServices]

or from slither import *

[BookOwl]

But that's considered bad practice. and we really shouldn't be encouraging that. What's wrong with having everything in the init.py

[CosmicWebServices]

I just realised we can put import slither in __init__.py

[ghost]

But why do we need this? And besides It's not like we have optional modules for slither.

[skistaddy]

@CosmicWebServices +1

[emmatyping]

But why do we need this? And besides It's not like we have optional modules for slither.

Python style dictates that as little operational code should go in an__init__.py file as possible. I believe the importing of slither.py, as @Omegabyte suggested is often the practice used.

[BookOwl]

I think that @Tymewalk needs to make this decision.

[CosmicWebServices]

OK

[Tymewalk]

I'm confused here.

So users will have to use from slither import *? Why not have a from slither import * in __init__.py and then use import slither like normal? Or just keeping things the way they are?

@ethanhs Where does it say that? I've never heard that rule, nor seen it used anywhere.

[CosmicWebServices]

no it will be the same as now

[Tymewalk]

@CosmicWebServices Oh, OK. But the code doesn't look that way.

[CosmicWebServices]

Well I made it in the format that __init__.py should be in but I would still test it...

[emmatyping]

I might have mis-spoken. What I really mean when I say "as little operational code as possible" is to say that the __init__.py should contain setup code.

The purpose of the init.py files is to include optional initialization code that runs as different levels of a package are encountered.

and

More often that not, it’s fine to just leave the init.py files empty. However, there are certain situations where they might include code. For example, an init.py file can be used to automatically load submodules

src

Also:

For a simple package, you might be tempted to throw utility methods, factories and exceptions into your init.py. Don’t. A well-formed init.py serves one very important purpose: to import from sub-modules.

src

One more:

init.py can be an empty file but it is often used to perform setup needed for the package(import things, load things into path, etc).

src

Based on these, I would recommend from . import slither or simply import slither and move all the code to slither.py

[skistaddy]

@ethanhs +1

[Tymewalk]

@ethanhs Oh, OK. That makes more sense.

Based on that I think we should restructure init.py so users can still use import slither, but so the code is in a separate file.

[CosmicWebServices]

yeah that makes more sense

[Tymewalk]

I'm going to close this pull request as it doesn't do this which is the eventual goal (unless you'd like to update it so it does do this, @CosmicWebServices).

[CosmicWebServices]

u do it

[Tymewalk]

@CosmicWebServices I can't since I don't have write access to your repo.

If you'd like I can close this and just make a new PR.

[CosmicWebServices]

PR?

[skistaddy]

@CosmicWebServices Pull Request

[CosmicWebServices]

What is in the new pull request???

[Tymewalk]

@CosmicWebServices I'd rewrite the current version of Slither to work so init.py only contains an import and users can still use import slither.

[CosmicWebServices]

OK I will do that tommarow

[Tymewalk]

@CosmicWebServices OK then.

This PR is being left open since @CosmicWebServices is working on updating it.

[CosmicWebServices]

Updated it!

[emmatyping]

I'm going to recommend that you do from . import slither, as that will not cause issues with namespaces getting imported twice etc.

Just an idea.

[CosmicWebServices]

Good Idea Fixing that

[CosmicWebServices]

Done

[Tymewalk]

Tests can still use import slither correctly, right?

[BookOwl]

I think that for this change to be accepted, all the tests must work perfectly without making any changes.

[CosmicWebServices]

@BookOwl I agree

[BookOwl]

@CosmicWebServices, what progress have you made?

[CosmicWebServices]

It is done

[BookOwl]

OK, @Tymewalk can look it over one more time and then we can merge.

[Tymewalk]

@BookOwl @CosmicWebServices I'm cloning that repo now to test.

[Tymewalk]

Used import slither in basicTest.py.

Traceback (most recent call last):
  File "slither/tests/basicTest.py", line 3, in <module>
    snakey = slither.Sprite()
AttributeError: 'module' object has no attribute 'Sprite'

[BookOwl]

@Tymewalk, I think that the __init__.py file needs to be changed to from .slither import *

[BookOwl]

Yep, that fixes it.

[BookOwl]

@CosmicWebServices, can you add that fix? Also, the tests need to be changed back to import slither, not from slither import *

[CosmicWebServices]

Got it...

[BookOwl]

I just tested it and it seems to work, but you need to revert the tests' import back to import slither Other than that, it looks great! Thanks for putting in all this work.

[CosmicWebServices]

I forgot about the tests I will do that soon (tomorrow probably)

[BookOwl]

@CosmicWebServices, when do you think that you will finish this?

[CosmicWebServices]

I forgot doing it now

[CosmicWebServices]

Done sorry it took so long!