Skip to content
This repository has been archived by the owner on Jan 31, 2022. It is now read-only.

Updates to packaging #108

Merged
merged 6 commits into from
Jun 5, 2018

Conversation

jsturdy
Copy link
Contributor

@jsturdy jsturdy commented Jun 4, 2018

Description

Few tweaks based on feedback of current packaged files

How Has This Been Tested?

Update to anautilities has not been, but is a semantic change only, a proof-of-principle of proper relative imports that will be extended to other package specific modules (where appropriate, e.g., not in tools that utilize a __main__).
The other update is non-functionality change.

@jsturdy
Copy link
Contributor Author

jsturdy commented Jun 4, 2018

Added also updated gembuild submodule for pip-only make targets for building/developing inside a virtualenv

@@ -0,0 +1,3 @@
importlib
setuptools>25,<=38
Copy link
Contributor

Choose a reason for hiding this comment

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

I used setuptools<38 to get make rpm to work with Python 2.6 (see here). It wasn't working with <39.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you know which version got pulled in?

Copy link
Contributor

@bdorney bdorney left a comment

Choose a reason for hiding this comment

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

Changes to imports in anautilities not understood at both functional level and rationale behind it.

@@ -66,7 +66,7 @@ def getDirByAnaType(anaType, cName, ztrim=4):
pass

# Check Paths
from gempython.utils.wrappers import envCheck
from ..utils.wrappers import envCheck
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably my ignorance here but I don't really follow what this (change to line 69) does; or the motivation for it?

I guess it's related to the discussion:

Update imports syntax in anautilities

Update to anautilities has not been, but is a semantic change only, a proof-of-principle of proper relative imports that will be extended to other package specific modules (where appropriate, e.g., not in tools that utilize a main).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

anautilities.py lives in gempython/gemplotting/
It wants to import from wrappers.py which lives in gempython/utils
Because anautilities.py will never be called standalone (no __main__), it is a "part of the package", so it should reference all imports from the gempython namespace relatively (wrt where it lives) so it imports from ..utils.wrappers (i.e., look in the package structure in the directory above where I am for utils/wrappers.py)
This type of import should (and can) only be done as deep as the package itself is (don't import .....somewhere.outside.of.gempython), and must not be done for anything that will be called either as a standalone, lives-on-the-PATH executable, nor for a module with a __main__ (executable via python -m <modulename>, as those pieces are not a "part" of the package in the same sense (and can be located outside of the package itself)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah so the import ..some.package

Is like calling cd ... I understand.

@jsturdy jsturdy merged commit 8bd6cac into cms-gem-daq-project:develop Jun 5, 2018
@jsturdy jsturdy deleted the updates-to-packaging branch June 5, 2018 12:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants