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

BUG: Fix bug extracting meas_date #10277

Merged
merged 2 commits into from
Feb 1, 2022
Merged

Conversation

larsoner
Copy link
Member

@larsoner larsoner commented Jan 31, 2022

Closes #10147

Turns out that localized dates is a pain in Python, and solving it in a general way requires a lot of work and/or additional dependencies. This implements a simpler solution: use a script to make a dict of translations and try them all. It's quick to execute, and easy enough to generate the dict of translations that we need.

@mlrocca feel free to try this branch (or just wait until it's merged into main and try that), it should fix your bug.

@rob-luke feel free to review and merge if you're happy

Copy link
Member

@agramfort agramfort left a comment

Choose a reason for hiding this comment

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

@rob-luke merge if happy

thx @larsoner

@hoechenberger
Copy link
Member

This is crazy!

And what about all other locales? Like, there must be dozens more?

@larsoner
Copy link
Member Author

larsoner commented Feb 1, 2022

Yes, there are many, many more locales. But we can deal with that later if we need to

@agramfort agramfort merged commit d53c5bb into mne-tools:main Feb 1, 2022
@agramfort
Copy link
Member

let's move on here. thx @larsoner

@hoechenberger
Copy link
Member

hoechenberger commented Feb 1, 2022

Thanks @larsoner!
My comment was not meant to urge you to add more locales now, I was merely interested to know if there'd maybe need to be more in the future :)

@larsoner larsoner deleted the nirstar branch February 1, 2022 14:49
@rob-luke
Copy link
Member

rob-luke commented Feb 1, 2022

Awesome! Thanks @larsoner

@mlrocca
Copy link

mlrocca commented Feb 1, 2022 via email

@larsoner
Copy link
Member Author

larsoner commented Feb 2, 2022

The easiest thing to do now that this has been merged is something like

pip install --upgrade https://github.com/mne-tools/mne-python/zipball/main

it will install the latest version of the main branch (1.0.dev0), where this PR was merged.

@mlrocca
Copy link

mlrocca commented Feb 15, 2022 via email

@larsoner
Copy link
Member Author

Not to NIRx, but you could try writing to SNIRF using MNE-NIRS

https://mne.tools/mne-nirs/generated/mne_nirs.io.snirf.write_raw_snirf.html#mne_nirs.io.snirf.write_raw_snirf

@mlrocca
Copy link

mlrocca commented Feb 16, 2022 via email

@rob-luke
Copy link
Member

Hi @mlrocca,

Exporting as nirx format is not something that is planned. The nirx format has been superseded by SNIRF as larsoner mentioned. New NIRX devices actually export as SNIRF natively now. Further, the nirx format is not documented and does not support exporting as OD or haemoglobin.

@mlrocca
Copy link

mlrocca commented Feb 17, 2022 via email

larsoner referenced this pull request in mne-tools/mne-testing-data Feb 17, 2022
@larsoner
Copy link
Member Author

@mlrocca this sounds more like a usage question, and we try to stick to code bugs and enhancement requests on GitHub. Could you please post your question to https://mne.discourse.group/ instead?

@mlrocca
Copy link

mlrocca commented Feb 17, 2022 via email

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.

add support for NIRStar 14.2 files
5 participants