nimi-python icon indicating copy to clipboard operation
nimi-python copied to clipboard

Consider redefining __module__ when re-exporting types in __init__.py

Open sbethur opened this issue 4 years ago • 0 comments

Description of issue

In __init__.py, we re-export the types contained in the module like:

from niscope.errors import DriverWarning  # noqa: F401
from niscope.errors import Error  # noqa: F401
from niscope.session import Session  # noqa: F401

from niscope.waveform_info import WaveformInfo  # noqa: F401

from niscope.waveform_info import struct_niScope_wfmInfo  # noqa: F401

This puts the types at package level, which is good. It allows users to do:

>>> import niscope
>>> niscope.Session

..even though Session is defined in session module. But repr() still includes the module:

>>> import niscope
>>> niscope.Session
<class 'niscope.session.Session'>
>>> niscope.WaveformInfo
<class 'niscope.waveform_info.WaveformInfo'>
>>> import nidigital
>>> nidigital.HistoryRAMCycleInformation
<class 'nidigital.history_ram_cycle_information.HistoryRAMCycleInformation'>

We should consider redefining __module__. Advantages of doing so (Copied from here):

  1. We tell clients to from nigitial import HistoryRAMCycleInformation, guaranteeing that this will work (up until we make an API breaking change). It logically follows the __module__ for that type would be nidigital.
  2. We would avoid exposing an implementation detail. If we moved where the class was defined are we sure we aren't going to break someone? Someone JSON-ifying the data, or pickling it? Setting __module__ to nidgitial means we can move the class as much as we want without worrying about exposing implementation details.
  3. This has the added bonus of "using" the class so no more suppressing F401 in __init__.py.
  4. Lastly, the new way leads to shorter strings.

sbethur avatar Apr 24 '20 22:04 sbethur