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

Better error message for BaseExtractor.load() #3170

Merged
merged 9 commits into from
Jul 12, 2024

Conversation

h-mayorquin
Copy link
Collaborator

As in the title, related to issue #3148

@h-mayorquin h-mayorquin self-assigned this Jul 9, 2024
@h-mayorquin h-mayorquin mentioned this pull request Jul 9, 2024
@alejoe91 alejoe91 added the core Changes to core module label Jul 9, 2024
@alejoe91 alejoe91 added this to the 0.101.0 milestone Jul 9, 2024
Copy link
Collaborator

@zm711 zm711 left a comment

Choose a reason for hiding this comment

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

Even with the grammar improvement I have no clue what the problem with the base_folder is. At least for the file_path you're saying that based on your tests it doesn't seem to exist. But just telling me that I gave a base_folder doesn't tell me how to fix my problem.

And the error rule for Heberto is it must be actionable.

src/spikeinterface/core/base.py Outdated Show resolved Hide resolved
@h-mayorquin
Copy link
Collaborator Author

Github seems to have some connection errors today.

@zm711
Copy link
Collaborator

zm711 commented Jul 9, 2024

I think pre-commit broke this :/

No--really we should have put parentheses around the string. That's my bad in my suggestion. I can't blame pre-commit.

@h-mayorquin
Copy link
Collaborator Author

Should be solved int the last commit.

@h-mayorquin h-mayorquin marked this pull request as ready for review July 9, 2024 21:41
@alejoe91 alejoe91 merged commit 4c473d1 into SpikeInterface:main Jul 12, 2024
15 checks passed
@h-mayorquin h-mayorquin deleted the better_error_in_load branch July 12, 2024 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Changes to core module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants