fix: make pyarrow an optional dependency post-3.20.0 yanked release

[tswast]

Partial rollback of 0ac6e9b

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

• Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
• Ensure the tests and linter pass
• Code coverage does not decrease (if any source code was changed)
• Appropriate docs were updated (if necessary)

Fixes #1877 🦕

[chalmerlowe]

@tswast

We appear to have two conflicting versions of pyarrow:

The noxfile.py file identifies these versions of Python:

Line 40: UNIT_TEST_PYTHON_VERSIONS = ["3.7", "3.8", "3.12"]

Later when we reference

Line 125: UNIT_TEST_PYTHON_VERSIONS[0]

we are implicitly referencing version 3.7 (at least until the definition of this constant changes) AND we then tell the system to install version 1.0.0 of pyarrow.
Line 126: session.install("pyarrow==1.0.0")

But in testing/constraints-3.7.txt we list the pyarrow version as

Line 30: pyarrow==3.0.0

QUESTION: should those two match?
QUESTION: if we are making pyarrow optional, why in the unit_noextras session are we requiring pyarrow?

[tswast]

QUESTION: should those two match?

No. They should not. testing/constraints-3.7.txt covers when pyarrow is installed as part of our package installation, e.g. unit-3.7. This doesn't apply to the unit_noextras-3.7 session.

QUESTION: if we are making pyarrow optional, why in the unit_noextras session are we requiring pyarrow?

There is a comment explaining exactly this. See: #933 Requiring the correct version of pyarrow to be installed broke some environments where pyarrow was stuck on an older version. We need to be robust to this failure mode.

[tswast]

See: https://pip.pypa.io/en/stable/user_guide/#constraints-files for more information on constraints files. When applied, it affects what version of a package is installed but it doesn't affect packages that aren't included because of a transitive dependency of the package being installed.

[Linchin]

LGTM. I will not merge for now, waiting for Chalmer to see the response.

[chalmerlowe]

Thanks @tswast
Took a look at the write-up on constraints.txt files.
My understanding is:

• unlike a requirements.txt file, it does NOT require that a specific package be installed
• HOWEVER, if that package DOES get installed (maybe as a dependency of something else OR as an optional package), it is constrained to what is allowed in the constraints.txt file.

[tswast]

That's correct. Note that we are installing pyarrow without a constraints file in unit_noextras and pyarrow is not included as a dependency in any subsequent installations, so the constraints do not apply.

[chalmerlowe]

LGTM

[shollyman]

Generally, looks like a good unwinding of the behavior changes and test configurations. I did add one question around test philosophy that may be outside the scope of this PR specifically.

[shollyman]

Is there a way of asserting that a test like this was run at least once across multiple sessions? Without that, we could still potentially swallow this signal if the precondition is never met.

[tswast]

Coverage should catch that. We check coverage on test files as well as the core library.

[tswast]

See:

python-bigquery/noxfile.py

Line 97 in 7dfee0c

"--cov=tests/unit",