Skip to content
This repository has been archived by the owner on Aug 29, 2020. It is now read-only.

Change the plasma module so that it's statically analysable #122

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Change the plasma module so that it's statically analysable #122

wants to merge 2 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Mar 16, 2020

Which provides code linting etc.

I added plasma/__init__.py which exposes all the contents of the files in plasma/ that were imported separately before. All imports have been changed to from plasma import *. For now I changed Plasma.py to PlasmaStubs.py to not conflict with the engine's Plasma module. Hoikas proposed changing the engine's module instead, to _Plasma (I didn't do that because I don't have engine development set up).

I also changed the Plasma stubs to raise NotImplementedError(), further improving code linting.

Copy link
Member

@Hoikas Hoikas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

python/plasma/__init__.py is missing the license block.

Edit: Also, it might be better to explicitly catch the ImportError, which is I think how several system modules handle this situation.

Can we make the imports relative eg from PlasmaTypes import *?

@ghost
Copy link
Author

ghost commented Mar 16, 2020

Amended.

@ghost
Copy link
Author

ghost commented Mar 16, 2020

I missed converting the import Plasma... cases (e.g. import PlasmaControlKeys), which were not analysing correctly as a result. Amended now.

@ghost
Copy link
Author

ghost commented Mar 16, 2020

Removed line ending changes, moved change to #123

@Hoikas
Copy link
Member

Hoikas commented Mar 17, 2020

Todo before this is merged: check for any naughty assumptions in plPythonPack and verify that packed python works after repacking with this.

@ghost
Copy link
Author

ghost commented May 4, 2020

This will probably be deferred until we're on Python 3, as we may want to use stub files for the plasma module.

@zrax zrax mentioned this pull request May 6, 2020
3 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant