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

Feature: DLomix integration #250

Merged
merged 172 commits into from
Sep 17, 2024
Merged

Conversation

JSchlensok
Copy link
Contributor

@JSchlensok JSchlensok commented Jul 26, 2024

PR Checklist

  • This comment contains a description of changes (with reason)
  • Referenced issue is linked
  • If you've fixed a bug or added code that should be tested, add tests!
  • Documentation in docs is updated

Description of changes

Enable off-line prediction of intensity using DLomix.

Technical details

  • refactor oktoberfest::pr::predict to abstract prediction using Koina/DLomix away from it into new oktoberfest::pr::dlomix and oktoberfest::pr::koina modules
  • add DLomix as optional dependency

Caveats

  • depends on code from DLomix and spectrum-fundamentals that's not released via PyPI, which requires special treatment with Nox
  • does not yet contain integration tests
  • DLomix logging is a mess, some of it can't easily be hidden from the user
  • early commits are a mess

Refactor the methods in oktoberfest::predict to use a predictor class
abstracted from Koina.

Add a new oktoberfest::predict::dlomix module with a bare-bones
interface for using DLomix models for local prediction of intensity.
- manually merge changes to predict functions from divergent development
branch
- rename predict::predict.py to predict::alignment.py to better reflect the
static methods left in it
- get rid of unnecessary imports
- consistently use generic type annotations from typing instead of
built-in ones
@JSchlensok JSchlensok requested a review from picciama July 26, 2024 14:41
@github-actions github-actions bot added the enhancement New feature or request label Jul 26, 2024
Copy link
Contributor

@picciama picciama left a comment

Choose a reason for hiding this comment

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

looking very nice already. Specifically the abstraction hopefully makes the whole prediction stuff a bit more easy to handle. A few things need clarification, see comments.

oktoberfest/runner.py Outdated Show resolved Hide resolved
oktoberfest/runner.py Outdated Show resolved Hide resolved
oktoberfest/runner.py Outdated Show resolved Hide resolved
oktoberfest/utils/config.py Show resolved Hide resolved
oktoberfest/predict/predictor.py Outdated Show resolved Hide resolved
oktoberfest/predict/dlomix.py Outdated Show resolved Hide resolved
oktoberfest/data/spectra.py Outdated Show resolved Hide resolved
oktoberfest/data/spectra.py Outdated Show resolved Hide resolved
oktoberfest/data/spectra.py Outdated Show resolved Hide resolved
docs/config.rst Outdated Show resolved Hide resolved
pyupgrade will otherwise reformat GenericType[] from typing to 3.10+
genericType[] if `from __future__ import annotations` is present
new spectrum_fundamentals & spectrum_io releases, new dlomix release in
development, updated dependencies
Refactor for legibility, actually use frag_dict,
enable keeping additional columns, remove decoys from data, add stub for
filtering by Andromeda score
Add sections where necessary, expand & split up some existing paragraphs
Add some cross-references
@picciama picciama force-pushed the feature/dlomix-integration branch 2 times, most recently from cf76872 to 395373f Compare September 13, 2024 13:39
@picciama picciama force-pushed the feature/dlomix-integration branch from 395373f to 3490afa Compare September 13, 2024 13:51
@JSchlensok JSchlensok merged commit 0ab857f into development Sep 17, 2024
26 checks passed
@JSchlensok JSchlensok deleted the feature/dlomix-integration branch September 17, 2024 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants