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

Automated code formatting #72

Merged
merged 13 commits into from
Aug 21, 2024
Merged

Automated code formatting #72

merged 13 commits into from
Aug 21, 2024

Conversation

nicolossus
Copy link
Member

@nicolossus nicolossus commented May 16, 2024

This PR adds automated code formatting and linting via pre-commit hooks. The PR also reformats the entire codebase and fix warnings from the linter.

(Note that the PR is against the bug-fixes branch, as this is the most recent.)

Copy link
Contributor

@alejoe91 alejoe91 left a comment

Choose a reason for hiding this comment

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

Hi @nicolossus

Cool! Please remove the test_data folder from linting though, because these are datasets generated by the acquisition systems.

Can you revert the changes on those files too?

@nicolossus
Copy link
Member Author

Cool! Please remove the test_data folder from linting though, because these are datasets generated by the acquisition systems.

Can you revert the changes on those files too?

@alejoe91 Fixed 👍

@nicolossus nicolossus requested a review from alejoe91 May 16, 2024 10:28
pyproject.toml Outdated
"ipywidgets>=8.1.1",
"nwbwidgets>=0.11.3",
"tbb>=2021.11.0",
"tbb>=2021.11.0", # TODO: pip can't find tbb or tbb4py (at least on macOS). Is it needed?
Copy link
Member Author

Choose a reason for hiding this comment

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

@alejoe91 @lepmik As far as I understand, tbb is a Linux only package. Having this here as a hard requirement limits installation on macOS for local experimentation or development. Is tbb really a necessary dependency?

Copy link
Contributor

Choose a reason for hiding this comment

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

this is supposed to run on educloud, and tbb is needed there

Copy link
Member Author

@nicolossus nicolossus May 16, 2024

Choose a reason for hiding this comment

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

I see. 8d8e408 proposes a solution that will let the package be installable on macOS with tbb listed in the requirements:

dependencies = [
    ...
    "tbb>=2021.11.0; platform_system != 'Darwin'",
]

pyproject.toml Outdated Show resolved Hide resolved
@nicolossus nicolossus merged commit 8483af7 into bug-fixes Aug 21, 2024
2 checks passed
@nicolossus nicolossus deleted the dev_nico branch August 21, 2024 12:56
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.

2 participants