[python] Add Windows support for the Python API #1811

teo-tsirpanis · 2023-10-19T22:30:07Z

Issue and/or context:

Changes:

Notes for Reviewer:

SC-35700

Fixes #1360.

shortcut-integration · 2023-10-19T22:30:11Z

This pull request has been linked to Shortcut Story #35700: Implement Windows support for SOMA.

…place.

This target corresponds to the dynamic libtiledbsoma library, but we already link to it through `TILEDB_SOMA_OBJECTS`.

eddelbuettel · 2023-10-25T12:27:38Z

libtiledbsoma/src/tiledbsoma/tiledbsoma

-// #include "soma_object.h"
-// #include "soma_sparse_ndarray.h"
-
+#include "tiledbsoma_export.h"


I presume that is a cmake generated file? Should we say so? A bit weird to include something not in the sources themselves but only in the build/ tree.

I will add a comment. We need this file to support exporting symbols in a cross-platform fashion (replacing this). The Core already has one.

Understood from what's in it but I think I have seen many other project have such #define statements in the normal include directories, no?

Thanks for dealing with the Windows build by the way. Much appreciated.

I have seen many other project have such #define statements in the normal include directories

We could do that, but having CMake generate the file automatically handles the varying platforms, compilers and static or dynamic linkage.

It now runs on `pull_request` instead of `push`, covering PRs from forks (added `workflow_dispatch` for those that might still want to run it without opening a PR), and adds a Windows matrix job (only for python 3.10 to reduce the jobs of the workflow). Also fixed two apparent typos.

codecov-commenter · 2023-10-25T17:33:07Z

Codecov Report

Merging #1811 (c4e871d) into main (04ab23e) will decrease coverage by 14.86%.
The diff coverage is n/a.

❗ Current head c4e871d differs from pull request most recent head 4570f96. Consider uploading reports for the commit 4570f96 to get more accurate results

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #1811       +/-   ##
===========================================
- Coverage   63.11%   48.26%   -14.86%     
===========================================
  Files         106       72       -34     
  Lines       10056     6458     -3598     
===========================================
- Hits         6347     3117     -3230     
+ Misses       3709     3341      -368

Flag	Coverage Δ
python	`?`
r	`48.26% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
python_api	`∅ <ø> (∅)`
libtiledbsoma	`∅ <ø> (∅)`

And avoid allocating temporary arrays in some cases.

teo-tsirpanis · 2023-11-20T14:04:09Z

The only Windows test failure was fixed by TileDB-Inc/TileDB-Py#1863. This is ready for review.

teo-tsirpanis · 2023-12-04T20:41:40Z

CI on Windows is green. Failures seem to be network-related; one address it showed in the error message gives 404 to me as well.

eddelbuettel · 2023-12-04T20:48:41Z

There was an entirely unrelated hickup in the CI we use for R, I just took care of that and am keeping fingers crossed for the re-run that is now happening.

nguyenv · 2023-12-05T00:39:28Z

apis/python/setup.py

@@ -151,7 +147,10 @@ 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.
-        subprocess.run(["./bld"], cwd=scripts_dir)
+        if os.name == "nt":
+            subprocess.run(["pwsh.exe", "./bld.ps1"], cwd=scripts_dir, check=True)


@beroy is currently working on removing the bld script and porting the cmake commands over into setup.py. I don't think anything needs to be modified in this particular PR but just letting relevant parties be aware.

nguyenv

Thanks!

johnkerl · 2023-12-06T14:46:54Z

Merging after discussion with @teo-tsirpanis and @nguyenv . cc @beroy re the additional pwsh.exe script along with bld for his setup.py refactoring.

This reverts commit ff491ab.

…ata#1811)" (single-cell-data#1959)" This reverts commit e231201.

* Revert "Revert "Add Windows support for the Python API (#1811)" (#1959)" This reverts commit e231201. * Roll back Windows CI.

teo-tsirpanis added 10 commits October 25, 2023 15:11

Call string() to convert a path to a string.

c4ea9c2

Use $<CONFIG> instead of ${CMAKE_BUILD_TYPE} in places easy to re…

be66890

…place.

Use the export header macros to export the LOG_*** functions.

8ebb6b9

Don't pass -Wno-deprecated-declarations in MSVC.

d15a79c

Fix an MSVC overflow warning that turns into an error.

01861b8

Don't link to the tiledbsoma target in the unit test.

5aa8dbe

This target corresponds to the dynamic libtiledbsoma library, but we already link to it through `TILEDB_SOMA_OBJECTS`.

Run Python CI on Windows.

b3dd472

Small fixes in the Python CI workflows.

e1566cb

Remove an outdated comment.

ba8be30

Fix including tiledbsoma_export.h.

f67d0a6

teo-tsirpanis force-pushed the windows branch from a05f6dd to f67d0a6 Compare October 25, 2023 12:12

eddelbuettel reviewed Oct 25, 2023

View reviewed changes

teo-tsirpanis force-pushed the windows branch from dbfa5e7 to b7a5a3b Compare October 25, 2023 15:48

teo-tsirpanis added 2 commits October 25, 2023 19:23

Fix include directories.

37862a2

Add missing headers.

de685d1

teo-tsirpanis force-pushed the windows branch from b7a5a3b to de685d1 Compare October 25, 2023 16:24

teo-tsirpanis added 5 commits October 25, 2023 22:06

Add a Powershell script to build libtiledbsoma.

271bfd8

Fix the build script.

5aa848c

Fix symlink path resolution.

13c5b98

Reformat.

4c7bac4

Import the tiledb library on Windows only.

09653e6

teo-tsirpanis force-pushed the windows branch from 69550b9 to 09653e6 Compare October 25, 2023 22:07

teo-tsirpanis added 4 commits October 26, 2023 01:24

Add Windows support in the Python code.

a68112b

Install the runtime dlls of tiledbsoma.

76a409d

Fix the lint job on Windows.

dc7bbac

Run more steps on bash.

d5be97b

teo-tsirpanis force-pushed the windows branch from 43fc4d0 to d5be97b Compare October 25, 2023 23:31

teo-tsirpanis and others added 2 commits November 8, 2023 18:01

Force creating the SOMA_JOINID column with type int64.

4743c8f

Merge branch 'main' into windows

7bfbab8

teo-tsirpanis marked this pull request as draft November 17, 2023 15:41

Ensure the coordinates are of type int64.

36ec168

And avoid allocating temporary arrays in some cases.

teo-tsirpanis force-pushed the windows branch from bcc0f01 to 36ec168 Compare November 17, 2023 23:28

teo-tsirpanis added 2 commits November 20, 2023 14:32

Explicitly set timestamps in the metadata tests.

be6b411

Add a delay of ten milliseconds to fix race conditions.

79cab88

teo-tsirpanis force-pushed the windows branch from 1cf43dc to 79cab88 Compare November 20, 2023 13:00

teo-tsirpanis marked this pull request as ready for review November 20, 2023 14:04

Merge branch 'main' into windows

f1c3e60

Delete apis/python/src/tiledbsoma/tiledbsoma.dll

4570f96

nguyenv reviewed Dec 5, 2023

View reviewed changes

nguyenv approved these changes Dec 5, 2023

View reviewed changes

johnkerl merged commit ff491ab into single-cell-data:main Dec 6, 2023
14 checks passed

teo-tsirpanis deleted the windows branch December 6, 2023 14:50

johnkerl added a commit that referenced this pull request Dec 6, 2023

Revert "Add Windows support for the Python API (#1811)"

bb3589d

This reverts commit ff491ab.

johnkerl mentioned this pull request Dec 6, 2023

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

Merged

johnkerl added a commit that referenced this pull request Dec 6, 2023

Revert "Add Windows support for the Python API (#1811)" (#1959)

e231201

This reverts commit ff491ab.

teo-tsirpanis added a commit to teo-tsirpanis/TileDB-SOMA that referenced this pull request Dec 6, 2023

Revert "Revert "Add Windows support for the Python API (single-cell-d…

22e3ae5

…ata#1811)" (single-cell-data#1959)" This reverts commit e231201.

teo-tsirpanis added a commit to teo-tsirpanis/TileDB-SOMA that referenced this pull request Dec 6, 2023

Revert "Revert "Add Windows support for the Python API (single-cell-d…

f2b4d48

…ata#1811)" (single-cell-data#1959)" This reverts commit e231201.

teo-tsirpanis mentioned this pull request Dec 6, 2023

[python] Add back Windows support for the Python API, but without CI #1961

Merged

johnkerl pushed a commit that referenced this pull request Dec 6, 2023

Add back Windows support for the Python API, but without CI. (#1961)

cf72782

* Revert "Revert "Add Windows support for the Python API (#1811)" (#1959)" This reverts commit e231201. * Roll back Windows CI.

johnkerl mentioned this pull request Dec 6, 2023

[python] Full Windows pre-push CI matrix #1962

Open

johnkerl changed the title ~~Add Windows support for the Python API~~ [python] Add Windows support for the Python API Dec 6, 2023

teo-tsirpanis mentioned this pull request Dec 7, 2023

[c++] Have tiledbsoma-cli link to tiledbsoma #1965

Merged

johnkerl mentioned this pull request May 23, 2024

Tracker for Windows support #2631

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[python] Add Windows support for the Python API #1811

[python] Add Windows support for the Python API #1811

teo-tsirpanis commented Oct 19, 2023 •

edited

Loading

shortcut-integration bot commented Oct 19, 2023

eddelbuettel Oct 25, 2023 •

edited

Loading

teo-tsirpanis Oct 25, 2023

eddelbuettel Oct 25, 2023

eddelbuettel Oct 25, 2023

teo-tsirpanis Oct 25, 2023

codecov-commenter commented Oct 25, 2023 •

edited

Loading

teo-tsirpanis commented Nov 20, 2023

teo-tsirpanis commented Dec 4, 2023

eddelbuettel commented Dec 4, 2023

nguyenv Dec 5, 2023

nguyenv left a comment

johnkerl commented Dec 6, 2023

[python] Add Windows support for the Python API #1811

[python] Add Windows support for the Python API #1811

Conversation

teo-tsirpanis commented Oct 19, 2023 • edited Loading

shortcut-integration bot commented Oct 19, 2023

eddelbuettel Oct 25, 2023 • edited Loading

Choose a reason for hiding this comment

teo-tsirpanis Oct 25, 2023

Choose a reason for hiding this comment

eddelbuettel Oct 25, 2023

Choose a reason for hiding this comment

eddelbuettel Oct 25, 2023

Choose a reason for hiding this comment

teo-tsirpanis Oct 25, 2023

Choose a reason for hiding this comment

codecov-commenter commented Oct 25, 2023 • edited Loading

Codecov Report

teo-tsirpanis commented Nov 20, 2023

teo-tsirpanis commented Dec 4, 2023

eddelbuettel commented Dec 4, 2023

nguyenv Dec 5, 2023

Choose a reason for hiding this comment

nguyenv left a comment

Choose a reason for hiding this comment

johnkerl commented Dec 6, 2023

teo-tsirpanis commented Oct 19, 2023 •

edited

Loading

eddelbuettel Oct 25, 2023 •

edited

Loading

codecov-commenter commented Oct 25, 2023 •

edited

Loading