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

Add functions to export feature data to CSV and JSON formats #390

Merged
merged 11 commits into from
May 8, 2024

Conversation

ilkilic
Copy link
Collaborator

@ilkilic ilkilic commented May 6, 2024

We have also eased up on type checking. Now, the system will try to convert the value to the correct type, such as int to float.

Checklist:

  • Unit tests are added to cover the changes (skip if not applicable).
  • The changes are mentioned in the documentation (skip if not applicable).
  • CHANGELOG file is updated (skip if not applicable).

@ilkilic ilkilic self-assigned this May 6, 2024
Copy link
Collaborator

@AurelienJaquier AurelienJaquier left a comment

Choose a reason for hiding this comment

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

I have a few comments :3

efel/settings.py Outdated Show resolved Hide resolved
tests/test_io.py Outdated
import efel
import os
import json
feature_values = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are we sure that this is exactly what efel would return? No numpy stuff?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also would be interesting to have some null/None values in it (not sure which one to sure exactly, use whatever efel is using)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point, I have switched to using the output from get_feature_values instead and tested with some more traces that have some or all None feature values

tests/test_io.py Outdated Show resolved Hide resolved
efel/io.py Outdated Show resolved Hide resolved
tests/test_settings.py Outdated Show resolved Hide resolved
@ilkilic ilkilic requested a review from anilbey May 7, 2024 14:25
tests/test_io.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@AurelienJaquier AurelienJaquier left a comment

Choose a reason for hiding this comment

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

Looks great, thank you Ilkan!

@ilkilic ilkilic merged commit 2380092 into master May 8, 2024
20 checks passed
@ilkilic ilkilic deleted the io-output branch May 8, 2024 08:54
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.

3 participants