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

[python] Revert "Add Windows support for the Python API" #1959

Merged
merged 1 commit into from
Dec 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .github/workflows/python-ci-full.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ jobs:
strategy:
fail-fast: false
matrix:
os: [ubuntu-22.04, macos-12, windows-2019]
# TODO: restore Windows build once we have C++/libtiledbsoma integration supported there
os: [ubuntu-22.04, macos-12]
# os: [ubuntu-22.04, macos-12, windows-2019]
python-version: ['3.7', '3.8', '3.9', '3.10', '3.11']
include:
- runs-on: ubuntu-22.04
Expand All @@ -35,6 +37,7 @@ jobs:
python_version: ${{ matrix.python-version }}
cc: ${{ matrix.cc }}
cxx: ${{ matrix.cxx }}
is_mac: ${{ contains(matrix.os, 'macos') }}
report_codecov: ${{ matrix.os == 'ubuntu-22.04' && matrix.python-version == '3.9' }}
run_lint: ${{ matrix.os == 'ubuntu-22.04' && matrix.python-version == '3.9' }}
secrets: inherit
15 changes: 5 additions & 10 deletions .github/workflows/python-ci-minimal.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,10 @@ name: TileDB-SOMA Python CI (Minimal)
# To test the full matrix on a working branch, invoke ./python-ci-full.yml from
# https://github.com/single-cell-data/TileDB-SOMA/actions/workflows/python-ci-full.yml
on:
pull_request:
branches:
- main
- 'release-*'
push:
paths-ignore:
- 'apis/r/**'
workflow_dispatch:


jobs:
build:
strategy:
Expand All @@ -24,20 +20,19 @@ jobs:
os: [ubuntu-22.04, macos-12]
python-version: ['3.10', '3.7']
include:
- os: ubuntu-22.04
- runs-on: ubuntu-22.04
cc: gcc-11
cxx: g++-11
- os: macos-12
- runs-on: macos-12
cc: clang
cxx: clang++
- os: windows-2019
python-version: '3.10'
uses: ./.github/workflows/python-ci-single.yml
with:
os: ${{ matrix.os }}
python_version: ${{ matrix.python-version }}
cc: ${{ matrix.cc }}
cxx: ${{ matrix.cxx }}
is_mac: ${{ contains(matrix.os, 'macos') }}
report_codecov: ${{ matrix.python-version == '3.10' }}
run_lint: ${{ matrix.python-version == '3.10' }}
secrets: inherit
15 changes: 8 additions & 7 deletions .github/workflows/python-ci-single.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,22 +14,26 @@ on:
required: true
type: string
cc:
required: false
required: true
type: string
cxx:
required: false
required: true
type: string
report_codecov:
required: true
type: boolean
is_mac:
required: false
type: boolean
default: false
run_lint:
required: false
type: boolean
default: false

jobs:
lint:
if: inputs.run_lint
if: ${{ inputs.run_lint }}
runs-on: ${{ inputs.os }}
steps:
- name: Checkout TileDB-SOMA
Expand All @@ -52,7 +56,6 @@ jobs:
run: python -m pip -v install pre-commit && pre-commit run -a -v

- name: Check C++ Format
shell: bash
run: ./scripts/run-clang-format.sh . clang-format 0 $(find libtiledbsoma -name "*.cc" -or -name "*.h")

build:
Expand All @@ -69,7 +72,7 @@ jobs:
if: ${{ inputs.os == 'macos-12' }}
run: sysctl -a | grep cpu
- name: Select XCode version
if: startsWith(inputs.os, 'macos')
if: inputs.is_mac
uses: maxim-lobanov/setup-xcode@v1
with:
# Pending https://github.com/actions/runner-images/issues/6350
Expand Down Expand Up @@ -123,14 +126,12 @@ jobs:
run: ctest --output-on-failure --test-dir build/libtiledbsoma -C Release --verbose

- name: Run pytests for C++
shell: bash
# Setting PYTHONPATH ensures the tests load the in-tree source code unde apis/python/src
# instead of copy we `pip install`ed to site-packages above. That's needed for the code
# coverage analysis to work.
run: PYTHONPATH=$(pwd)/apis/python/src python -m pytest --cov=apis/python/src --cov-report=xml libtiledbsoma/test -v --durations=20

- name: Run pytests for Python
shell: bash
# Setting PYTHONPATH ensures the tests load the in-tree source code unde apis/python/src
# instead of copy we `pip install`ed to site-packages above. That's needed for the code
# coverage analysis to work.
Expand Down
1 change: 0 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
.vscode*
.vs
*.swp
dist/*
**/venv
Expand Down
32 changes: 14 additions & 18 deletions apis/python/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,13 @@

if libtiledbsoma_dir is None:
scripts_dir = this_dir / "dist_links" / "scripts"
scripts_dir = scripts_dir.resolve()

libtiledbsoma_dir = scripts_dir.parent / "dist"
if scripts_dir.is_symlink():
# in git source tree
libtiledbsoma_dir = this_dir.parent.parent / "dist"
else:
# in extracted sdist, with libtiledbsoma copied into dist_links/
libtiledbsoma_dir = this_dir / "dist_links" / "dist"
else:
libtiledbsoma_dir = pathlib.Path(libtiledbsoma_dir)

Expand Down Expand Up @@ -147,10 +151,7 @@ def find_or_build_package_data(setuptools_cmd):
# cause that cache to fall out of sync.
#
# See `.github/workflows/python-ci-single.yml` for configuration.
if os.name == "nt":
subprocess.run(["pwsh.exe", "./bld.ps1"], cwd=scripts_dir, check=True)
else:
subprocess.run(["./bld"], cwd=scripts_dir, check=True)
subprocess.run(["./bld"], cwd=scripts_dir)
lib_dir = libtiledbsoma_exists()
assert lib_dir, "error when building libtiledbsoma from source"

Expand Down Expand Up @@ -197,13 +198,10 @@ def run(self):
str(libtiledbsoma_dir / "lib"),
str(tiledb_dir / "lib"),
]

CXX_FLAGS = []

if os.name != "nt":
CXX_FLAGS.append(f'-Wl,-rpath,{str(libtiledbsoma_dir / "lib")}')
CXX_FLAGS.append(f'-Wl,-rpath,{str(tiledb_dir / "lib")}')

CXX_FLAGS = [
f'-Wl,-rpath,{str(libtiledbsoma_dir / "lib")}',
f'-Wl,-rpath,{str(tiledb_dir / "lib")}',
]
if sys.platform == "darwin":
CXX_FLAGS.append("-mmacosx-version-min=10.14")

Expand All @@ -227,7 +225,7 @@ def run(self):
setuptools.setup(
name="tiledbsoma",
description="Python API for efficient storage and retrieval of single-cell data using TileDB",
long_description=open("README.md", encoding="utf-8").read(),
long_description=open("README.md").read(),
long_description_content_type="text/markdown",
author="TileDB, Inc.",
author_email="help@tiledb.io",
Expand All @@ -244,7 +242,6 @@ def run(self):
"Operating System :: Unix",
"Operating System :: POSIX :: Linux",
"Operating System :: MacOS :: MacOS X",
"Operating System :: Microsoft :: Windows",
"Programming Language :: Python",
"Programming Language :: Python :: 3.7",
"Programming Language :: Python :: 3.8",
Expand All @@ -261,10 +258,9 @@ def run(self):
["src/tiledbsoma/pytiledbsoma.cc"],
include_dirs=INC_DIRS,
library_dirs=LIB_DIRS,
libraries=["tiledbsoma"] + (["tiledb"] if os.name == "nt" else []),
libraries=["tiledbsoma"],
extra_link_args=CXX_FLAGS,
extra_compile_args=["-std=c++17" if os.name != "nt" else "/std:c++17"]
+ CXX_FLAGS,
extra_compile_args=["-std=c++17"] + CXX_FLAGS,
language="c++",
)
],
Expand Down
7 changes: 2 additions & 5 deletions apis/python/src/tiledbsoma/_dataframe.py
Original file line number Diff line number Diff line change
Expand Up @@ -691,11 +691,8 @@ def _canonicalize_schema(
raise ValueError("DataFrame requires one or more index columns")

if SOMA_JOINID in schema.names:
joinid_type = schema.field(SOMA_JOINID).type
if joinid_type != pa.int64():
raise ValueError(
f"{SOMA_JOINID} field must be of type Arrow int64 but is {joinid_type}"
)
if schema.field(SOMA_JOINID).type != pa.int64():
raise ValueError(f"{SOMA_JOINID} field must be of type Arrow int64")
else:
# add SOMA_JOINID
schema = schema.append(pa.field(SOMA_JOINID, pa.int64()))
Expand Down
6 changes: 3 additions & 3 deletions apis/python/src/tiledbsoma/_general_utilities.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"""General utility functions.
"""

import platform
import os
import sys

import tiledb
Expand Down Expand Up @@ -64,5 +64,5 @@ def show_package_versions() -> None:
)
print("libtiledbsoma version() ", libtiledbsoma_version())
print("python version ", ".".join(str(v) for v in sys.version_info))
u = platform.uname()
print("OS version ", u.system, u.release)
u = os.uname()
print("OS version ", u.sysname, u.release)
10 changes: 3 additions & 7 deletions apis/python/src/tiledbsoma/_read_iters.py
Original file line number Diff line number Diff line change
Expand Up @@ -478,14 +478,10 @@ def _coords_strider(
elif isinstance(coords, int):
coords = np.array([coords], dtype=np.int64)
elif isinstance(coords, Sequence):
coords = np.array(coords, dtype=np.int64)
coords = np.array(coords).astype(np.int64)
elif isinstance(coords, (pa.Array, pa.ChunkedArray)):
coords = coords.to_numpy().astype(np.int64, copy=False)
elif isinstance(coords, np.ndarray):
coords = coords.astype(np.int64)
elif isinstance(coords, slice):
pass
else:
coords = coords.to_numpy()
elif not isinstance(coords, (np.ndarray, slice)):
raise TypeError("Unsupported slice coordinate type")

if isinstance(coords, slice):
Expand Down
4 changes: 1 addition & 3 deletions apis/python/src/tiledbsoma/_tiledb_array.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@

def _load_libs() -> None:
"""Loads the required TileDB-SOMA native library."""
if os.name == "nt":
lib_name = "tiledbsoma.dll"
elif sys.platform == "darwin":
if sys.platform == "darwin":
lib_name = "libtiledbsoma.dylib"
else:
lib_name = "libtiledbsoma.so"
Expand Down
2 changes: 1 addition & 1 deletion apis/python/src/tiledbsoma/io/ingest.py
Original file line number Diff line number Diff line change
Expand Up @@ -1163,7 +1163,7 @@ def _write_dataframe(
else:
df.rename(columns={"index": id_column_name}, inplace=True)

df[SOMA_JOINID] = np.asarray(axis_mapping.data, dtype=np.int64)
df[SOMA_JOINID] = np.asarray(axis_mapping.data)

df.set_index(SOMA_JOINID, inplace=True)

Expand Down
5 changes: 0 additions & 5 deletions apis/python/tests/test_factory.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
from time import sleep
from typing import Type

import numpy as np
Expand Down Expand Up @@ -73,8 +72,6 @@ def tiledb_object_uri(tmp_path, object_type, metadata_typename, encoding_version
)
def test_open(tiledb_object_uri, expected_soma_type: Type):
"""Happy path tests"""
# TODO: Fix Windows test failures without the following.
sleep(0.01)
soma_obj = soma.open(tiledb_object_uri)
assert isinstance(soma_obj, expected_soma_type)
typed_soma_obj = soma.open(tiledb_object_uri, soma_type=expected_soma_type)
Expand Down Expand Up @@ -135,8 +132,6 @@ def test_open_wrong_type(tiledb_object_uri, wrong_type):
)
def test_factory_unsupported_version(tiledb_object_uri):
"""All of these should raise, as they are encoding formats from the future"""
# TODO: Fix Windows test failures without the following.
sleep(0.01)
with pytest.raises(ValueError):
soma.open(tiledb_object_uri)

Expand Down
22 changes: 9 additions & 13 deletions apis/python/tests/test_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ def test_metadata(soma_object):
# Verify the metadata is empty to start. "Empty" defined as no keys
# other than soma_ keys.
uri = soma_object.uri
timestamp = soma_object.tiledb_timestamp_ms
with soma_object:
assert non_soma_metadata(soma_object) == {}
non_soma_keys = [k for k in soma_object.metadata if not k.startswith("soma_")]
Expand All @@ -71,7 +70,7 @@ def test_metadata(soma_object):
with pytest.raises(soma.SOMAError):
soma_object.metadata["x"] = "y"

with _factory.open(uri, "r", tiledb_timestamp=timestamp) as read_obj:
with _factory.open(uri, "r") as read_obj:
assert non_soma_metadata(read_obj) == {"foobar": True}
assert "foobar" in read_obj.metadata
# Double-check the various getter methods
Expand All @@ -83,15 +82,15 @@ def test_metadata(soma_object):
with pytest.raises(soma.SOMAError):
read_obj.metadata["x"] = "y"

with _factory.open(uri, "w", tiledb_timestamp=timestamp + 1) as second_write:
with _factory.open(uri, "w") as second_write:
second_write.metadata.update(stay="frosty", my="friends")
assert non_soma_metadata(second_write) == {
"foobar": True,
"stay": "frosty",
"my": "friends",
}

with _factory.open(uri, "w", tiledb_timestamp=timestamp + 2) as third_write:
with _factory.open(uri, "w") as third_write:
del third_write.metadata["stay"]
third_write.metadata["my"] = "enemies"
assert non_soma_metadata(third_write) == {"foobar": True, "my": "enemies"}
Expand Down Expand Up @@ -125,49 +124,46 @@ def test_add_delete_metadata(soma_object):

def test_delete_add_metadata(soma_object):
uri = soma_object.uri
timestamp = soma_object.tiledb_timestamp_ms
with soma_object:
soma_object.metadata["hdyfn"] = "destruction"
assert non_soma_metadata(soma_object) == {"hdyfn": "destruction"}

with _factory.open(uri, "w", tiledb_timestamp=timestamp + 1) as second_write:
with _factory.open(uri, "w") as second_write:
assert non_soma_metadata(second_write) == {"hdyfn": "destruction"}
del second_write.metadata["hdyfn"]
assert non_soma_metadata(second_write) == {}
second_write.metadata["hdyfn"] = "somebody new"
assert non_soma_metadata(second_write) == {"hdyfn": "somebody new"}

with _factory.open(uri, "r", tiledb_timestamp=timestamp + 1) as reader:
with _factory.open(uri, "r") as reader:
assert non_soma_metadata(reader) == {"hdyfn": "somebody new"}


def test_set_set_metadata(soma_object):
uri = soma_object.uri
timestamp = soma_object.tiledb_timestamp_ms

with soma_object:
soma_object.metadata["content"] = "content"

with _factory.open(uri, "w", tiledb_timestamp=timestamp + 1) as second_write:
with _factory.open(uri, "w") as second_write:
second_write.metadata["content"] = "confidence"
second_write.metadata["content"] = "doubt"

with _factory.open(uri, "r", tiledb_timestamp=timestamp + 1) as reader:
with _factory.open(uri, "r") as reader:
assert non_soma_metadata(reader) == {"content": "doubt"}


def test_set_delete_metadata(soma_object):
uri = soma_object.uri
timestamp = soma_object.tiledb_timestamp_ms

with soma_object:
soma_object.metadata["possession"] = "obsession"

with _factory.open(uri, "w", tiledb_timestamp=timestamp + 1) as second_write:
with _factory.open(uri, "w") as second_write:
second_write.metadata["possession"] = "funny thing about opinions"
del second_write.metadata["possession"]

with _factory.open(uri, "r", tiledb_timestamp=timestamp + 1) as reader:
with _factory.open(uri, "r") as reader:
assert non_soma_metadata(reader) == {}


Expand Down
2 changes: 1 addition & 1 deletion libtiledbsoma/cmake/Modules/FindSpdlog_EP.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ if (NOT SPDLOG_FOUND)
-DCMAKE_PREFIX_PATH=${EP_INSTALL_PREFIX}
-DCMAKE_INSTALL_PREFIX=${EP_INSTALL_PREFIX}
-DCMAKE_OSX_ARCHITECTURES=${CMAKE_OSX_ARCHITECTURES}
-DCMAKE_BUILD_TYPE=$<CONFIG>
-DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}
-DCMAKE_CXX_COMPILER=${CMAKE_CXX_COMPILER}
-DCMAKE_C_COMPILER=${CMAKE_C_COMPILER}
-DCMAKE_TOOLCHAIN_FILE=${CMAKE_TOOLCHAIN_FILE}
Expand Down
Loading