Skip to content

allow mixed str and dict in lint config#3228

Merged
mashehu merged 25 commits into
nf-core:devfrom
mashehu:fix-yml-config-type
Dec 3, 2024
Merged

allow mixed str and dict in lint config#3228
mashehu merged 25 commits into
nf-core:devfrom
mashehu:fix-yml-config-type

Conversation

@mashehu

@mashehu mashehu commented Oct 17, 2024

Copy link
Copy Markdown
Contributor

we should probably rewrite this into a TypedDict or similar in the future.

@Joon-Klaps

Joon-Klaps commented Oct 17, 2024

Copy link
Copy Markdown
Contributor

Could you write a quick test for this as well?

Joon-Klaps
Joon-Klaps reviewed Oct 22, 2024
View reviewed changes

@Joon-Klaps Joon-Klaps left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Tried my best but am not entirely following with all changes due to a lack of knowledge on the codebase of nf-tools.
Tested it also again on my pipeline & new failed tests were popping up because actions_ci was not included in the class NFCoreYamlLintConfig(BaseModel

Comment thread tests/pipelines/lint/test_nfcore_yml.py
Comment thread nf_core/utils.py Outdated
Comment thread nf_core/utils.py
@mashehu mashehu added the WIP Work in progress label Oct 22, 2024
@ewels ewels added this to the 3.1 milestone Nov 21, 2024
@mashehu
Merge branch 'dev' of github.com:nf-core/tools into fix-yml-config-type
c63f3be 
# Conflicts:
#	CHANGELOG.md
#	nf_core/pipelines/create/create.py
@codecov

codecov Bot commented Dec 2, 2024 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 84.21053% with 6 lines in your changes missing coverage. Please review.

Project coverage is 75.87%. Comparing base (dabf1ef) to head (eed0598).
Report is 52 commits behind head on dev.

Files with missing lines Patch % Lines
nf_core/pipelines/lint/nfcore_yml.py 76.92% 3 Missing ⚠️
nf_core/pipelines/sync.py 78.57% 3 Missing ⚠️
Additional details and impacted files

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mashehu mashehu removed the WIP Work in progress label Dec 3, 2024
mirpedrol
mirpedrol approved these changes Dec 3, 2024
View reviewed changes

@mirpedrol mirpedrol left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

a couple of small suggestions, otherwise LGTM

Comment thread nf_core/pipelines/lint/__init__.py Outdated
Comment thread nf_core/pipelines/lint/readme.py Outdated
@mashehu @mirpedrol
Apply suggestions from code review
eed0598 
Co-authored-by: Júlia Mir Pedrol <mirp.julia@gmail.com>
@mashehu mashehu enabled auto-merge December 3, 2024 11:03
@mashehu mashehu merged commit a7351a2 into nf-core:dev Dec 3, 2024
@Joon-Klaps

Joon-Klaps commented Dec 3, 2024

Copy link
Copy Markdown
Contributor

Thanks @mashehu ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Joon-Klaps Joon-Klaps Joon-Klaps left review comments

@mirpedrol mirpedrol mirpedrol approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

3.1

Development

Successfully merging this pull request may close these issues.

5 participants

@mashehu @Joon-Klaps @mirpedrol @ewels @nf-core-bot