Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "refactor(api): Move opentrons.initialize() to its own file (#15191)" #15234

Merged
merged 1 commit into from
May 21, 2024

Conversation

SyntaxColoring
Copy link
Contributor

@SyntaxColoring SyntaxColoring commented May 21, 2024

Overview

PR #15191 / commit 1be63cd sneakily broke hardware-testing because of a circular import problem.

python -m pipenv run python -m hardware_testing.liquid_sense --simulate --pipette 1000 --channels 1
Traceback (most recent call last):
  File "/home/ryan/.pyenv/versions/3.10.2/lib/python3.10/runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/home/ryan/.pyenv/versions/3.10.2/lib/python3.10/runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "/home/ryan/opentrons/hardware-testing/hardware_testing/liquid_sense/__main__.py", line 12, in <module>
    from hardware_testing.opentrons_api import helpers_ot3
  File "/home/ryan/opentrons/hardware-testing/hardware_testing/opentrons_api/helpers_ot3.py", line 19, in <module>
    from opentrons.config.robot_configs import build_config_ot3, load_ot3 as load_ot3_config
  File "/home/ryan/opentrons/api/src/opentrons/config/robot_configs.py", line 9, in <module>
    from . import CONFIG, defaults_ot3, defaults_ot2, gripper_config, feature_flags as ff
  File "/home/ryan/opentrons/api/src/opentrons/config/defaults_ot3.py", line 5, in <module>
    from opentrons.hardware_control.types import OT3AxisKind, InstrumentProbeType
  File "/home/ryan/opentrons/api/src/opentrons/hardware_control/__init__.py", line 12, in <module>
    from .adapters import SynchronousAdapter
  File "/home/ryan/opentrons/api/src/opentrons/hardware_control/adapters.py", line 6, in <module>
    from .protocols import AsyncioConfigurable
  File "/home/ryan/opentrons/api/src/opentrons/hardware_control/protocols/__init__.py", line 10, in <module>
    from .liquid_handler import LiquidHandler
  File "/home/ryan/opentrons/api/src/opentrons/hardware_control/protocols/liquid_handler.py", line 6, in <module>
    from .instrument_configurer import InstrumentConfigurer
  File "/home/ryan/opentrons/api/src/opentrons/hardware_control/protocols/instrument_configurer.py", line 10, in <module>
    from opentrons.hardware_control.instruments.ot2.pipette import Pipette
  File "/home/ryan/opentrons/api/src/opentrons/hardware_control/instruments/__init__.py", line 2, in <module>
    from .ot3.gripper import Gripper
  File "/home/ryan/opentrons/api/src/opentrons/hardware_control/instruments/ot3/gripper.py", line 18, in <module>
    from .instrument_calibration import (
  File "/home/ryan/opentrons/api/src/opentrons/hardware_control/instruments/ot3/instrument_calibration.py", line 7, in <module>
    from opentrons.config.robot_configs import (
ImportError: cannot import name 'default_pipette_offset' from partially initialized module 'opentrons.config.robot_configs' (most likely due to a circular import) (/home/ryan/opentrons/api/src/opentrons/config/robot_configs.py)
make: *** [Makefile:160: test-liquid-sense] Error 1

This does a git revert on it until we can figure out a proper solution.

Test Plan

Review requests

Generalizing from that specific stack trace, the problem seems to be:

  • Some things in opentrons.hardware_control import some things in opentrons.config
  • Some things in opentrons.config import some things in opentrons.hardware_control
  • Because both opentrons.hardware_control and opentrons.config have a bunch of __init__.py re-exports, you can't import anything from either of them without dragging in the whole subtree, and this is what creates the cycle

This looks kind of hairy to solve. I think removing the re-exports is a good practice in general, but that could get intrusive.

I'm open to other ideas. Maybe it's backwards that opentrons.config is depending on opentrons.hardware_control in the first place? Maybe we want opentrons.config to be more self-contained, at the cost of duplicating some types with opentrons.hardware_control?

Risk assessment

Low.

@SyntaxColoring SyntaxColoring requested a review from a team as a code owner May 21, 2024 16:00
Copy link
Contributor

@ryanthecoder ryanthecoder left a comment

Choose a reason for hiding this comment

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

thanks, can help look into this more later today

@SyntaxColoring SyntaxColoring merged commit f63483d into edge May 21, 2024
27 checks passed
@SyntaxColoring SyntaxColoring deleted the revert_pr_15191 branch May 21, 2024 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants