SainsburyWellcomeCentre / aeon_mecha

Project Aeon's main library for interfacing with acquired data. Contains modules for raw data file io, data querying, data processing, data qc, database ingestion, and building computational data pipelines.
BSD 3-Clause "New" or "Revised" License
6 stars 6 forks source link

Enhancements and refactoring: Ruff checks resolved #443

Closed MilagrosMarin closed 3 days ago

MilagrosMarin commented 3 weeks ago

This PR closes #441 and #417 by removing the exclusion of ruff checks for dj_pipeline from pyproject.toml.

This is a summary of the relevant changes that have been implemented, with all checks passing:

Resolved issues for aeon/dj_pipeline:

Additional fixes to the rest of the reporitory:

In addition to the above changes, I have also resolved most of the ruff lint issues from the ignored list in pyproject.toml, ensuring all checks passed:

Deprecation Fix:

MilagrosMarin commented 3 weeks ago

@ttngu207 I’ve incorporated all the changes we discussed during the PR review.

jkbhagatio commented 2 weeks ago

Can we re- remove ignoring ruff d104 and d100 errors in the pyproject.toml in this PR as well?

lochhh commented 2 weeks ago

Can we re- remove ignoring ruff d104 and d100 errors in the pyproject.toml in this PR as well?

wdym? I thought this was already the case? These are all that's left 🎉 https://github.com/SainsburyWellcomeCentre/aeon_mecha/blob/48493cbc0de8b6e2a5e20bcebc0f30f3f7e6719b/pyproject.toml#L99-L106

MilagrosMarin commented 1 week ago

@lochhh and @ttngu207 , thank you both so much for your reviews! I’ve completed the third round of review for this PR. Since there were many new suggestions in different forms, could you please @lochhh give it a final review? Thank you!

ttngu207 commented 3 days ago

Thanks @MilagrosMarin for addressing all comments. I made another PR here with some final suggestions. And just one last comment about noqa B023 here:

https://github.com/SainsburyWellcomeCentre/aeon_mecha/blob/6279bda3faa16916a2a84226e9ee82e669a88367/aeon/io/reader.py#L164

If it's too much trouble to test this we can also keep the current change.

I have tested this, good to go here

ttngu207 commented 3 days ago

All comments/suggestions have been addressed/resolved PR merged