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

Integration with dandelion #240

Closed
wants to merge 12 commits into from

Conversation

zktuong
Copy link
Contributor

@zktuong zktuong commented Feb 9, 2021

Some initial code that should work with dandelion to convert between the two instances from zktuong/dandelion#49 onwards.

Still need to work in the extra bits on fully initialized scirpy and dandelion objects.

Adjustment to allow for reading from pandas dataframe
add dandelion conversion wrapper
Copy link
Collaborator

@grst grst left a comment

Choose a reason for hiding this comment

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

Added some comments, I hope I'm not too nitpicky ;)

Eventually, we'll need to add tests for that as well, but you can also leave that to me.

scirpy/io/_convert_anndata.py Outdated Show resolved Hide resolved
scirpy/io/_convert_anndata.py Outdated Show resolved Hide resolved
scirpy/io/_io.py Outdated Show resolved Hide resolved
scirpy/io/_io.py Outdated Show resolved Hide resolved
scirpy/io/_io.py Outdated Show resolved Hide resolved
scirpy/io/_io.py Outdated Show resolved Hide resolved
@grst
Copy link
Collaborator

grst commented Feb 10, 2021

Ah, and you'll need to run the black formatter as well.
The easiest way to ensure that is to use the pre commit hook - it will format on every git commit.

To set it up, run the following commands in the root of the repository

pip install pre-commit
pre-commit install

@grst grst added this to In progress in scirpy-dev Feb 17, 2021
@zktuong
Copy link
Contributor Author

zktuong commented Mar 5, 2021

This is almost ready for review. I'm just trying to solve some bugs with my github actions on dandelion's side holding up further work; seems like the actual tests on this side are working for now, pending updates to #241, but the docs are failing - seems unrelated?.

@grst
Copy link
Collaborator

grst commented Mar 5, 2021

The docs complain about something with the type annotations... don't see how this would be related since no type annotations have changed in this PR.

I'll finally dedicate some time to scirpy next week, but I would like to finish up #230 first, before I look into #241.
Is this blocking anything on your side?

@zktuong
Copy link
Contributor Author

zktuong commented Mar 5, 2021

Nope all good - take your time.

@grst
Copy link
Collaborator

grst commented Mar 21, 2021

This is now merged into #241. I'll keep you posted!

@grst grst closed this Mar 21, 2021
scirpy-dev automation moved this from In progress to Done Mar 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
scirpy-dev
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants