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

fix: Fix meta comparison and wildcard issue #113

Merged
merged 33 commits into from
Sep 11, 2024
Merged

Conversation

fxwiegand
Copy link
Contributor

@fxwiegand fxwiegand commented Aug 29, 2024

This PR activates meta_comparison for testing and contains some fixes for it by @Addimator. I also altered a path that was causing issues with wildcards in the goatools enrichment rule.

@fxwiegand fxwiegand marked this pull request as ready for review September 9, 2024 07:19
@fxwiegand fxwiegand changed the title test: Activate meta_comparison for testing fix: Fix meta comparison and wildcard issue Sep 9, 2024
Copy link
Collaborator

@dlaehnemann dlaehnemann left a comment

Choose a reason for hiding this comment

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

Nice cleanup and great to have tests running on this functionality.

Also, I started digging into polars (vs. pandas) functionality a bit, and found some spots where we might be able to clean up the syntax and avoid switching back to pandas. But if the suggestions don't simply work, it might make sense to get back to Adrian (and possibly Johannes), about some of the questions, as there might have been conscious decisions against using a particular polars functionality...

workflow/scripts/compare_enrichment.py Outdated Show resolved Hide resolved
workflow/scripts/compare_enrichment.py Outdated Show resolved Hide resolved
workflow/scripts/compare_enrichment.py Outdated Show resolved Hide resolved
workflow/scripts/compare_enrichment.py Outdated Show resolved Hide resolved
workflow/scripts/compare_pathways.py Outdated Show resolved Hide resolved
workflow/scripts/compare_pathways.py Outdated Show resolved Hide resolved
workflow/scripts/compare_pathways.py Outdated Show resolved Hide resolved
workflow/scripts/compare_pathways.py Outdated Show resolved Hide resolved
@fxwiegand
Copy link
Contributor Author

@dlaehnemann I think we are ready for a final review now 😊

@fxwiegand fxwiegand enabled auto-merge (squash) September 11, 2024 06:56
Copy link
Collaborator

@dlaehnemann dlaehnemann left a comment

Choose a reason for hiding this comment

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

As before, this looks good. Thanks for bearing with me and for teaching me some polars in the process. I have some minor suggestions and questions. Feel free to accept and address or dismiss, as you see fit.

workflow/scripts/compare_diffexp.py Show resolved Hide resolved
workflow/scripts/compare_enrichment.py Show resolved Hide resolved
workflow/scripts/compare_enrichment.py Show resolved Hide resolved
workflow/scripts/compare_enrichment.py Show resolved Hide resolved
workflow/scripts/compare_pathways.py Show resolved Hide resolved
workflow/scripts/compare_pathways.py Show resolved Hide resolved
workflow/scripts/compare_pathways.py Show resolved Hide resolved
@fxwiegand fxwiegand merged commit 3f8f126 into main Sep 11, 2024
6 checks passed
@fxwiegand fxwiegand deleted the fxwiegand-patch-1 branch September 11, 2024 14:12
@dlaehnemann
Copy link
Collaborator

Ah, didn't see the auto-merge... 🙈

dlaehnemann pushed a commit that referenced this pull request Sep 11, 2024
This PR does some minor refactoring in the meta comparison code as they
were still some comments from @dlaehnemann in #113 that havent been
addressed.
johanneskoester pushed a commit that referenced this pull request Sep 12, 2024
🤖 I have created a release *beep* *boop*
---


##
[2.7.2](v2.7.1...v2.7.2)
(2024-09-11)


### Bug Fixes

* Fix meta comparison and wildcard issue
([#113](#113))
([3f8f126](3f8f126))
* Fix meta comparison model and label path
([#111](#111))
([27b9cb1](27b9cb1))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

3 participants