Fix incorrect OR operand order when initializing Metaflow instance

[anurag99sai-alt]

The current implementation initializes the Metaflow instance as:

self._metaflow = Metaflow(_current_metadata) or _metaflow

Since Metaflow(_current_metadata) always returns a truthy object,
the _metaflow operand is never evaluated. This prevents reuse of
an existing Metaflow instance.

Swapping the operands ensures that an existing _metaflow instance
is reused when available:

self._metaflow = _metaflow or Metaflow(_current_metadata)

[greptile-apps]

Greptile Summary

This PR fixes a longstanding bug in MetaflowObject.__init__ where the expression Metaflow(_current_metadata) or _metaflow always evaluated to the freshly-constructed Metaflow object (which is always truthy), making the _metaflow parameter effectively dead code. Swapping the operands to _metaflow or Metaflow(_current_metadata) correctly reuses an existing instance when one is provided, consistent with the identical pattern already used for _current_namespace in the same method.

The PR also removes the Python 2 cPickle compatibility shim (try: import cPickle as pickle except: import pickle), replacing it with a direct import pickle. Metaflow has dropped Python 2 support, so the removal is appropriate.

Key changes:

• metaflow/client/core.py line 283 — swap OR operands so _metaflow or Metaflow(_current_metadata) is evaluated left-to-right, reusing an injected instance instead of always discarding it.
• metaflow/client/core.py lines 43–49 — delete Python 2 cPickle fallback; Python 3's pickle is now imported directly.

Minor observations:

• No new tests are added to cover the corrected reuse behaviour; a unit test asserting that a provided _metaflow instance is not replaced would prevent future regressions.
• When _metaflow is provided together with a non-None _current_metadata, the new code silently ignores _current_metadata. This is the intended semantics (the injected instance takes precedence), but it is worth documenting in the parameter docstring so callers are not surprised.

Confidence Score: 4/5

• Safe to merge — the logic fix is obviously correct and no existing behaviour is regressed for the common call paths.
• The OR-operand swap is a minimal, well-reasoned fix that aligns with the existing _current_namespace or get_namespace() pattern in the same method. All internal callers that pass _metaflow=self._metaflow continue to work correctly; external callers that pass only _current_metadata (the public API) also continue to work because _metaflow defaults to None. The only reason for not scoring 5 is the absence of a regression test for the fixed behaviour.
• No files require special attention.

Important Files Changed

Filename	Overview
metaflow/client/core.py	Two independent changes: (1) correct OR operand order so an existing _metaflow instance is reused instead of always discarded, and (2) drop Python 2 cPickle fallback in favour of a direct import pickle. Both changes are logically sound.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["MetaflowObject.__init__ called"] --> B{"_metaflow\nprovided?"}
    B -- "Yes (truthy Metaflow instance)" --> C["self._metaflow = _metaflow\n(reuse existing instance)"]
    B -- "No (None / falsy)" --> D["self._metaflow = Metaflow(_current_metadata)\n(create new instance)"]
    C --> E["Proceed with init\n(set _parent, _attempt, namespace…)"]
    D --> E

    style C fill:#22bb44,color:#fff
    style D fill:#2288cc,color:#fff

Loading

_{Last reviewed commit: 6e3ea73}