[python] Allow specification of X names on ingest [release-1.6] #1996

johnkerl · 2024-01-04T21:51:01Z

Issue and/or context: #1993 for the release-1.6 branch

Changes:

Notes for Reviewer:

codecov · 2024-01-04T21:57:53Z

Codecov Report

Merging #1996 (2afbd8c) into release-1.6 (8a7b523) will not change coverage.
The diff coverage is 100.00%.

Additional details and impacted files

@@             Coverage Diff              @@
##           release-1.6    #1996   +/-   ##
============================================
  Coverage        63.11%   63.11%           
============================================
  Files              106      106           
  Lines            10062    10062           
============================================
  Hits              6351     6351           
  Misses            3711     3711

Flag	Coverage Δ
python	`89.77% <100.00%> (ø)`
r	`48.28% <ø> (ø)`

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

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

johnkerl · 2024-01-08T14:56:38Z

Note: the 2nd commit be8c808 on this PR is a port of #1972's pin of TileDB-R from release-1.5 (which it is known-good) to release-1.6.

eddelbuettel · 2024-01-08T15:05:29Z

.github/workflows/r-ci.yml

+          cd apis/r
+          wget https://github.com/TileDB-Inc/TileDB-R/archive/refs/tags/0.22.0.tar.gz
+          Rscript -e 'remotes::install_deps("0.22.0.tar.gz")'
+          R CMD INSTALL 0.22.0.tar.gz    # as 0.22.0 is needed by SOMA 1.6


The idea of 'controlled download' was to do just this, but via binaries. Paul and I put some work into this. But let's see if brute works.

@eddelbuettel in fact this has already worked on release-1.5.

@eddelbuettel regarding controlled downgrades -- last week on #1993 despite multiple debug commits over a full day of debugging I was unable to decipher why the controlled-downgrades approach from #1994 was not working on MacOS. Since I was unable to get the pattern from #1994 applied successfully, I am now attempting to apply the successful pattern from #1972.

I did the same in December, believe it or not, but stopped when I saw no light on either end of any tunnel.

eddelbuettel · 2024-01-08T15:06:09Z

.github/workflows/r-ci.yml

+          cd apis/r
+          wget https://github.com/TileDB-Inc/TileDB-R/archive/refs/tags/0.22.0.tar.gz
+          Rscript -e 'remotes::install_deps("0.22.0.tar.gz")'
+          R CMD INSTALL 0.22.0.tar.gz    # as 0.22.0 is needed by SOMA 1.6


You should not need to do it twice, you can can skip the update.packages(ask=FALSE) too. That was from a different time fixing a different issue.

@eddelbuettel I went through multiple CI iterations on #1972 on the release-1.5 branch to find, and use, a known-good pattern which we could then apply for subsequent PRs on subsequent branches.

This PR is a such a subsequent PR on a subsequent branch (release-1.6).

eddelbuettel

See the comment inline. @mojaveazure raised the same point, correctly, and may want to chime in.

It's good that it builds now -- big step forward. But we don't need to build it twice.

eddelbuettel · 2024-01-08T15:27:09Z

.github/workflows/r-ci.yml

@@ -91,6 +87,13 @@ jobs:
      - name: Update Packages
        run: Rscript -e 'update.packages(ask=FALSE)'


I recommend to remove this block. It would allow you to remove the next block (ie lines 90 to 95 as well). And cut CI time down by a few minutes.

/CC @mojaveazure

@eddelbuettel trying commit number 3: de92e13

@eddelbuettel:

Commit 2 be8c808 had green CI

There were 3 of 3 CI fails with commit number 3: de92e13

Commit 4 5a76da1 reverts to commit 2 and has green CI

If I look at the correct page not three for three. One, plus two cancels. We know coverage is more fragile.

https://github.com/single-cell-data/TileDB-SOMA/actions/runs/7449708671/job/20267723363

@eddelbuettel I watched each one as I ran them.

Commit 3 had three fails

Commit 4 had a cancel (I don't know why), then a successful re-run

@eddelbuettel I am happy to re-run the already-green CI another time for commit number 4.

Am I looking in the wrong place?

And

Anyway, I have no beef with this but it may be a good idea to get someone else to look at this.

.github/workflows/r-ci.yml

johnkerl · 2024-01-08T19:01:06Z

@mojaveazure CI fail with

Symbol not found: (_tiledb_group_get_member_by_index_v2)

(2.18 vs 2.19) on commit number 5: 3ce3836

I am again reverting to commits 4 and 2 (which are identical) -- this will be a commit number 6 -- 2afbd8c -- using the proven-good approach for those commits.

mojaveazure

Dissapointing that the controlled update didn't work; it seems like our best bet is the update + reinstall

eddelbuettel · 2024-01-08T19:18:12Z

😿 Sometimes blunt tools are all we have.

Also worth reiterating that AFAICT all this work 'merely' lets us get by in CI, and may do little for users of SOMA who still fight similar battles.

johnkerl · 2024-01-08T19:33:34Z

Also worth reiterating that AFAICT all this work 'merely' lets us get by in CI, and may do little for users of SOMA who still fight similar battles.

@eddelbuettel indeed: as per our established procedure there are three things:

Green CI, here
Conda, which is easier because we can pin
End users doing non-Conda installs, for whom https://github.com/single-cell-data/TileDB-SOMA/wiki/R-quick-start

This PR addresses only the first of these three; as it is intended to do.

eddelbuettel · 2024-01-08T20:13:35Z

I just reiterated things internally in a conversartion, it is not ideal how things are set up because it makes builds from source harder than they maybe could be. You are fighting a heroic battle with the build system, yet there are still more possible build permutations behind other curtains. It's hard and complex ...

Thanks for getting this one finished!

Allow specification of X names on ingest [release-1.6]

e5f6a22

johnkerl mentioned this pull request Jan 8, 2024

[python] Allow specification of X names on ingest #1993

Merged

Apply #1972 to release-1.6

be8c808

eddelbuettel reviewed Jan 8, 2024

View reviewed changes

johnkerl marked this pull request as ready for review January 8, 2024 15:08

johnkerl requested a review from eddelbuettel January 8, 2024 15:10

eddelbuettel reviewed Jan 8, 2024

View reviewed changes

johnkerl added 2 commits January 8, 2024 10:39

code-review feedback

de92e13

revert previous commit

5a76da1

johnkerl requested review from eddelbuettel, aaronwolen and mojaveazure January 8, 2024 17:20

mojaveazure reviewed Jan 8, 2024

View reviewed changes

.github/workflows/r-ci.yml Show resolved Hide resolved

code-review feedback

3ce3836

Revert commit number 6, back to 2 and 4 (which are the same)

2afbd8c

mojaveazure approved these changes Jan 8, 2024

View reviewed changes

johnkerl merged commit 8728744 into release-1.6 Jan 8, 2024
14 checks passed

johnkerl deleted the kerl/X-name-release-1.6 branch January 8, 2024 19:33

johnkerl mentioned this pull request Jan 8, 2024

[python] Minor update to from_h5ad online help #1999

Merged

johnkerl changed the title ~~Allow specification of X names on ingest [release-1.6]~~ [python] Allow specification of X names on ingest [release-1.6] Jan 8, 2024

johnkerl mentioned this pull request Jan 9, 2024

[r] tiledbsoma 1.6.2 #2006

Merged

johnkerl mentioned this pull request Feb 20, 2024

[r/ci] Controlled downgrade for TileDB-R 0.23 #2157

Merged

johnkerl mentioned this pull request Mar 21, 2024

[r/ci] Controlled downgrade for TileDB-R 0.24 #2295

Merged

johnkerl mentioned this pull request Apr 3, 2024

[r/ci] Controlled downgrade for TileDB-R 0.25 #2373

Merged

johnkerl mentioned this pull request Apr 17, 2024

[r] Use lower and upper pin bounds on tiledb-r for release-1.9 branch #2374

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[python] Allow specification of X names on ingest [release-1.6] #1996

[python] Allow specification of X names on ingest [release-1.6] #1996

johnkerl commented Jan 4, 2024

codecov bot commented Jan 4, 2024 •

edited

Loading

johnkerl commented Jan 8, 2024

eddelbuettel Jan 8, 2024

johnkerl Jan 8, 2024

johnkerl Jan 8, 2024

eddelbuettel Jan 8, 2024

eddelbuettel Jan 8, 2024

johnkerl Jan 8, 2024

eddelbuettel left a comment

eddelbuettel Jan 8, 2024

johnkerl Jan 8, 2024 •

edited

Loading

johnkerl Jan 8, 2024

eddelbuettel Jan 8, 2024

johnkerl Jan 8, 2024 •

edited

Loading

johnkerl Jan 8, 2024

eddelbuettel Jan 8, 2024

johnkerl commented Jan 8, 2024 •

edited

Loading

mojaveazure left a comment

eddelbuettel commented Jan 8, 2024

johnkerl commented Jan 8, 2024 •

edited

Loading

eddelbuettel commented Jan 8, 2024

		@@ -91,6 +87,13 @@ jobs:
		- name: Update Packages
		run: Rscript -e 'update.packages(ask=FALSE)'

[python] Allow specification of X names on ingest [release-1.6] #1996

[python] Allow specification of X names on ingest [release-1.6] #1996

Conversation

johnkerl commented Jan 4, 2024

codecov bot commented Jan 4, 2024 • edited Loading

Codecov Report

johnkerl commented Jan 8, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eddelbuettel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnkerl Jan 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnkerl Jan 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnkerl commented Jan 8, 2024 • edited Loading

mojaveazure left a comment

Choose a reason for hiding this comment

eddelbuettel commented Jan 8, 2024

johnkerl commented Jan 8, 2024 • edited Loading

eddelbuettel commented Jan 8, 2024

codecov bot commented Jan 4, 2024 •

edited

Loading

johnkerl Jan 8, 2024 •

edited

Loading

johnkerl Jan 8, 2024 •

edited

Loading

johnkerl commented Jan 8, 2024 •

edited

Loading

johnkerl commented Jan 8, 2024 •

edited

Loading