Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Add pymde to dimensionality reduction #767

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.

Sign up for GitHub

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

Jump to bottom
Merged
scottgigante-immunai merged 12 commits into from
Jan 11, 2023
Merged

Add pymde to dimensionality reduction #767

scottgigante-immunai merged 12 commits into from
Jan 11, 2023

Conversation

@scottgigante-immunai
Copy link
Collaborator

@scottgigante-immunai scottgigante-immunai commented Jan 4, 2023 •
edited
Loading

Submission type

  • This submission adds a new method

Testing

Submission guidelines

  • This submission follows the guidelines in our
    Contributing document
  • I have checked to ensure there aren't other open Pull Requests for the
    same update/change

PR review checklist

This PR will be evaluated on the basis of the following checks:

  • The task addresses a valid open problem in single-cell analysis
  • The latest version of master is merged and tested
  • The methods/metrics are imported to __init__.py and were tested in the pipeline
  • Method and metric decorators are annotated with paper title, year, author, code
    version, and date
  • The README gives an outline of the methods, metrics and datasets in the folder

@scottgigante-immunai scottgigante-immunai marked this pull request as draft January 4, 2023 22:37
@scottgigante-immunai scottgigante-immunai marked this pull request as ready for review January 5, 2023 02:34
lazappi
lazappi reviewed Jan 9, 2023
View reviewed changes
openproblems/tasks/dimensionality_reduction/methods/pymde.py Outdated Show resolved Hide resolved
lazappi
lazappi approved these changes Jan 9, 2023
View reviewed changes
Copy link
Member

@lazappi lazappi left a comment

Choose a reason for hiding this comment

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

Looks good to me apart from the small comment on the method definitions. I guess #768 should be merged first though.

lazappi
lazappi requested changes Jan 10, 2023
View reviewed changes
Copy link
Member

@lazappi lazappi left a comment

Choose a reason for hiding this comment

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

I think this needs to be added to the README as well (sorry, missed that before)

@scottgigante-immunai
Copy link
Collaborator Author

scottgigante-immunai commented Jan 10, 2023

Ah. We haven't been doing that for any of the other tasks -- is it something we want to continue with DR?

@lazappi
Copy link
Member

lazappi commented Jan 10, 2023

Ah. We haven't been doing that for any of the other tasks -- is it something we want to continue with DR?

🤷🏻 It could be useful but you should also be able to see what methods are implemented from the website. I'm not particularly attached to it, I think it was in the template I was given so I tried to keep it up to date. Either we keep updating it or remove it, having it half done is probably the worst option.

@scottgigante-immunai
Copy link
Collaborator Author

scottgigante-immunai commented Jan 10, 2023

@LuckyMD thoughts? I moved it to the API section because it wasn't in the other tasks and it made the website very long. I think I'm inclined to let people go to the bibliography if they want details?

@LuckyMD
Copy link
Collaborator

LuckyMD commented Jan 10, 2023

Hmm.. it looks like a lot of effort went into that README that we're mostly not using on the website. I wouldn't make it the norm for other tasks, but I would probably keep it here given how much is already there... seems a waste to remove, no?

@scottgigante-immunai
Copy link
Collaborator Author

scottgigante-immunai commented Jan 11, 2023

I agree it seems like a waste to remove, but it could also be a waste to maintain something we don't want or need.

@lazappi
Copy link
Member

lazappi commented Jan 11, 2023

I agree it seems like a waste to remove, but it could also be a waste to maintain something we don't want or need.

That's what I was thinking. If we don't have it for the other tasks and there's no use for it going forward it should probably be removed to avoid maintaining it (although it did take me a while to write it all in the first place).

@LuckyMD
Copy link
Collaborator

LuckyMD commented Jan 11, 2023

fair enough. I hope you could use it for the supplementary materials @lazappi.

@scottgigante-immunai
Copy link
Collaborator Author

scottgigante-immunai commented Jan 11, 2023

I'm going to go ahead and merge this one, and we can remove the method descriptions from the API in a separate PR.

@codecov
Copy link

codecov bot commented Jan 11, 2023

Codecov Report

Base: 94.78% // Head: 94.78% // Decreases project coverage by -0.00% ⚠️

Coverage data is based on head (8e97631) compared to base (3d8964a).
Patch coverage: 96.38% of modified lines in pull request are covered.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #767      +/-   ##
==========================================
- Coverage   94.78%   94.78%   -0.01%     
==========================================
  Files         155      155              
  Lines        4163     4273     +110     
  Branches      215      225      +10     
==========================================
+ Hits         3946     4050     +104     
- Misses        142      147       +5     
- Partials       75       76       +1
Flag Coverage Δ
unittests 94.78% <96.38%> (-0.01%) ⬇️

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

Impacted Files Coverage Δ
...oblems/data/Wagner_2018_zebrafish_embryo_CRISPR.py 0.00% <ø> (ø)
openproblems/data/multimodal/citeseq.py 100.00% <ø> (ø)
openproblems/data/tabula_muris_senis.py 83.11% <ø> (ø)
...ration/batch_integration_embed/metrics/cc_score.py 100.00% <ø> (ø)
...n/batch_integration_embed/metrics/iso_label_sil.py 100.00% <ø> (ø)
...ntegration/batch_integration_embed/metrics/kBET.py 100.00% <ø> (ø)
...integration/batch_integration_embed/metrics/pcr.py 100.00% <ø> (ø)
...ation/batch_integration_embed/metrics/sil_batch.py 100.00% <ø> (ø)
...tion/batch_integration_embed/metrics/silhouette.py 100.00% <ø> (ø)
...tion/batch_integration_feature/methods/baseline.py 100.00% <ø> (ø)
... and 85 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@scottgigante-immunai scottgigante-immunai merged commit 2dba827 into openproblems-bio:main Jan 11, 2023
@scottgigante-immunai scottgigante-immunai deleted the dimensionality_reduction/mde branch January 11, 2023 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lazappi lazappi lazappi requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@scottgigante-immunai @lazappi @LuckyMD
Morty Proxy This is a proxified and sanitized view of the page, visit original site.