Skip to content

Commit

Permalink
Improve import structure of podio to feel more pythonic (#484)
Browse files Browse the repository at this point in the history
* Improve import structure of podio to feel more pythonic

* Make sure python bindings are usable without libs

* Make import of test_utils more robust

* Remove test_utils from imported modules

* Make Frame python bindings more robust

* Keep test_utils in podio module to make tests work
  • Loading branch information
tmadlener authored Sep 18, 2023
1 parent d402f74 commit 0cf68a8
Show file tree
Hide file tree
Showing 13 changed files with 81 additions and 17 deletions.
2 changes: 1 addition & 1 deletion .github/scripts/pylint.rc
Original file line number Diff line number Diff line change
Expand Up @@ -349,4 +349,4 @@ analyse-fallback-blocks=no

# Exceptions that will emit a warning when being caught. Defaults to
# "Exception"
overgeneral-exceptions=Exception
overgeneral-exceptions=builtins.Exception
4 changes: 2 additions & 2 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@ spack*

# Populated by cmake before build
/include/podio/podioVersion.h
/python/podio/__init__.py
/python/podio/__version__.py

# CLion artifacts
.idea/
cmake-build*/
cmake-build*/
4 changes: 2 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,8 @@ configure_file(${CMAKE_CURRENT_SOURCE_DIR}/podioVersion.in.h
${CMAKE_CURRENT_SOURCE_DIR}/include/podio/podioVersion.h )
install(FILES ${CMAKE_CURRENT_SOURCE_DIR}/include/podio/podioVersion.h
DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/podio )
configure_file(${CMAKE_CURRENT_SOURCE_DIR}/python/__init__.py.in
${CMAKE_CURRENT_SOURCE_DIR}/python/podio/__init__.py)
configure_file(${CMAKE_CURRENT_SOURCE_DIR}/python/__version__.py.in
${CMAKE_CURRENT_SOURCE_DIR}/python/podio/__version__.py)

#--- add license files ---------------------------------------------------------
install(FILES ${CMAKE_CURRENT_SOURCE_DIR}/NOTICE
Expand Down
File renamed without changes.
48 changes: 48 additions & 0 deletions python/podio/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
"""Python module for the podio EDM toolkit and its utilities"""
import sys

from .__version__ import __version__

from .podio_config_reader import * # noqa: F403, F401

import ROOT # pylint: disable=wrong-import-order

# Track whether we were able to dynamially load the library that is built by
# podio and enable certain features of the bindings only if they are actually
# available.
_DYNAMIC_LIBS_LOADED = False

# Check if we can locate the dictionary wthout loading it as this allows us to
# silence any ouptput. If we can find it, we can also safely load it
if ROOT.gSystem.DynamicPathName("libpodioDict.so", True):
ROOT.gSystem.Load("libpodioDict.so")
from ROOT import podio

_DYNAMIC_LIBS_LOADED = True

if _DYNAMIC_LIBS_LOADED:
from .frame import Frame
from . import root_io, sio_io, reading

from . import EventStore

try:
# For some reason the test_utils only work at (test) runtime if they are
# imported with the rest of podio. Otherwise they produce a weird c++ error.
# This happens even if we import the *exact* same file.
from . import test_utils # noqa: F401
except ImportError:
pass

# Make sure that this module is actually usable as podio even though most of
# it is dynamically populated by cppyy
sys.modules["podio"] = podio

__all__ = [
"__version__",
"Frame",
"root_io",
"sio_io",
"reading",
"EventStore"
]
14 changes: 11 additions & 3 deletions python/podio/frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,16 @@
import cppyy

import ROOT

# NOTE: It is necessary that this can be found on the ROOT_INCLUDE_PATH
ROOT.gInterpreter.LoadFile('podio/Frame.h') # noqa: E402
from ROOT import podio # noqa: E402 # pylint: disable=wrong-import-position
#
# We check whether we can actually load the header to not break python bindings
# in environments with *ancient* podio versions
if ROOT.gInterpreter.LoadFile('podio/Frame.h') == 0: # noqa: E402
from ROOT import podio # noqa: E402 # pylint: disable=wrong-import-position
_FRAME_HEADER_AVAILABLE = True
else:
_FRAME_HEADER_AVAILABLE = False


def _determine_supported_parameter_types():
Expand Down Expand Up @@ -45,7 +52,8 @@ def _determine_cpp_type(idx_and_type):
return tuple(zip(cpp_types, py_types))


SUPPORTED_PARAMETER_TYPES = _determine_supported_parameter_types()
if _FRAME_HEADER_AVAILABLE:
SUPPORTED_PARAMETER_TYPES = _determine_supported_parameter_types()


def _get_cpp_types(type_str):
Expand Down
3 changes: 2 additions & 1 deletion python/podio/test_EventStoreRoot.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@

from ROOT import TFile

from test_EventStore import EventStoreBaseTestCaseMixin # pylint: disable=import-error

from podio.EventStore import EventStore
from podio.test_EventStore import EventStoreBaseTestCaseMixin


class EventStoreRootTestCase(EventStoreBaseTestCaseMixin, unittest.TestCase):
Expand Down
5 changes: 3 additions & 2 deletions python/podio/test_EventStoreSio.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@
import unittest
import os

from test_utils import SKIP_SIO_TESTS # pylint: disable=import-error
from test_EventStore import EventStoreBaseTestCaseMixin # pylint: disable=import-error

from podio.EventStore import EventStore
from podio.test_EventStore import EventStoreBaseTestCaseMixin
from podio.test_utils import SKIP_SIO_TESTS


@unittest.skipIf(SKIP_SIO_TESTS, "no SIO support")
Expand Down
4 changes: 3 additions & 1 deletion python/podio/test_Frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@

import unittest

# pylint: disable=import-error
from test_utils import ExampleHitCollection # noqa: E402

from podio.frame import Frame
# using root_io as that should always be present regardless of which backends are built
from podio.root_io import Reader

from podio.test_utils import ExampleHitCollection

# The expected collections in each frame
EXPECTED_COLL_NAMES = {
Expand Down
3 changes: 2 additions & 1 deletion python/podio/test_ReaderRoot.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@

import unittest

from test_Reader import ReaderTestCaseMixin, LegacyReaderTestCaseMixin # pylint: disable=import-error

from podio.root_io import Reader, LegacyReader
from podio.test_Reader import ReaderTestCaseMixin, LegacyReaderTestCaseMixin


class RootReaderTestCase(ReaderTestCaseMixin, unittest.TestCase):
Expand Down
4 changes: 2 additions & 2 deletions python/podio/test_ReaderSio.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@

import unittest

from podio.test_Reader import ReaderTestCaseMixin, LegacyReaderTestCaseMixin
from podio.test_utils import SKIP_SIO_TESTS
from test_Reader import ReaderTestCaseMixin, LegacyReaderTestCaseMixin # pylint: disable=import-error
from test_utils import SKIP_SIO_TESTS # pylint: disable=import-error


@unittest.skipIf(SKIP_SIO_TESTS, "no SIO support")
Expand Down
3 changes: 2 additions & 1 deletion python/podio/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@

import os
import ROOT

ROOT.gSystem.Load("libTestDataModelDict.so") # noqa: E402
from ROOT import ExampleHitCollection, ExampleClusterCollection # noqa: E402 # pylint: disable=wrong-import-position

from podio.frame import Frame # pylint: disable=wrong-import-position
from podio import Frame # pylint: disable=wrong-import-position


SKIP_SIO_TESTS = os.environ.get("SKIP_SIO_TESTS", "1") == "1"
Expand Down
4 changes: 3 additions & 1 deletion tests/root_io/write_frame_root.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
"""Script to write a Frame in ROOT format"""

from podio import test_utils
from podio.root_io import Writer

from podio.root_io import Writer # pylint: disable=wrong-import-position


test_utils.write_file(Writer, "example_frame_with_py.root")

0 comments on commit 0cf68a8

Please sign in to comment.