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

TSV-ify the download images #257

Merged
merged 2 commits into from
Jan 29, 2024
Merged

Conversation

sjspielman
Copy link
Member

Closes #238

This PR updates all current download images (not including merged) to reflect that all our metadata files are .tsv, not .csv.

There is one additional change here - the spaceranger report along the way got renamed with a -; it was library_spaceranger_summary.html, now library_spaceranger-summary.html. I didn't notice this in #238, but saw it here when I filed the PR. I think it's a good move with other naming changes. The other consequences of this are we have to update the name in scpca-nf (quick fix), and we'd also have to update the file names in the Portal (I guess we can manually rename in prod and include in handoff). Or, we can revert the image back to an _.

Since the reports include the cell type reports, we should only release these updated images with the cell type release, not anndata. So, I'll cherry pick into the cell type release PR once this goes into development. Also, I would note in that CHANGELOG that the spatial summary report name has been tweaked.

@allyhawkins
Copy link
Member

The other consequences of this are we have to update the name in scpca-nf (quick fix), and we'd also have to update the file names in the Portal (I guess we can manually rename in prod and include in handoff). Or, we can revert the image back to an _.

I'm fine with making this name change, but given that we aren't going to include these docs changes until cell type annotation, anything that's already handed off can stay the way it is. I don't want to manually rename since there's like 100 samples of spatial, but I would be fine with including this in the release of scpca-nf with the bug fixes (AlexsLemonade/scpca-nf#669) and re-running that project before we hand off cell typing.

@sjspielman
Copy link
Member Author

but given that we aren't going to include these docs changes until cell type annotation, anything that's already handed off can stay the way it is.

Yes, definitely.

I would be fine with including this in the release of scpca-nf with the bug fixes (AlexsLemonade/scpca-nf#669) and re-running that project before we hand off cell typing.

I'll make an issue in admin that blocks the release.

Copy link
Member

@allyhawkins allyhawkins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before I approve this:

@sjspielman
Copy link
Member Author

sjspielman commented Jan 29, 2024

Copy link
Member

@allyhawkins allyhawkins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, although I don't think changing the file name of the spatial report to have - instead of _ means it should be in the changelog, but I think that's a question for @dvenprasad.

@sjspielman
Copy link
Member Author

but I think that's a question for @dvenprasad.

Stay tuned!

@sjspielman
Copy link
Member Author

Ok, it's a no, so I'll revert that commit!

@sjspielman sjspielman merged commit 1227fe9 into development Jan 29, 2024
2 checks passed
@sjspielman sjspielman deleted the sjspielman/238-tsv-images branch January 29, 2024 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants