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

[MAINT] Utils library Qt-less refactor (Part 1 of many) #881

Merged
merged 27 commits into from
Feb 7, 2022

Conversation

gabrielbmotta
Copy link
Collaborator

@gabrielbmotta gabrielbmotta commented Jan 11, 2022

This goes over a majority of the utils library and adds a std lib api (and a std lib backend) to most classes and static functions.

  • Overloaded functions with Qt-type parameters and return values with std lib versions.
  • Changes internal logic where applicable to use std lib containers/classes
  • When faced with functions with no input parameters and a qt return type, overloaded added and commented out.

Also:

  • min c++ version set to c++ 17 for use of filesystem library. Is this ok? this is a 5 year old standard at this point.
    (reverted, was causing issues with ubuntu 18 and osx 10.15 and lower std lib version. No longer using filesystem lib. )

  • Because adding more code results in a lower code coverage percentage even though the functions I overloaded were not being tested anyway (I think), I also wrote a test for circular buffer to bump up code coverage.

What is missing:

  • Going over threading use of QtConcurrent::mapReduced in spectrogram.h/cpp. Somewhat relates to the now-dormant time frequency PR WIP [ENH] Time Frequency Plugins for MNE Scan and MNE Analyze #785
  • QSemaphore use in circular buffer. Std lib of semaphore is c++20, and that is perhaps too recent a cut off point. We could implement our own semaphore with the same api as the c++20 version to make later replacement easy.

@juangpc
Copy link
Collaborator

juangpc commented Jan 11, 2022

great!

@codecov
Copy link

codecov bot commented Jan 11, 2022

Codecov Report

Merging #881 (937633a) into main (bd31738) will increase coverage by 6.08%.
The diff coverage is 0.74%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #881      +/-   ##
==========================================
+ Coverage   30.20%   36.28%   +6.08%     
==========================================
  Files         452      195     -257     
  Lines       39208    11805   -27403     
==========================================
- Hits        11841     4284    -7557     
+ Misses      27367     7521   -19846     
Impacted Files Coverage Δ
libraries/utils/file.cpp 0.00% <ø> (ø)
libraries/utils/ioutils.cpp 23.68% <0.00%> (-6.32%) ⬇️
libraries/utils/kmeans.cpp 0.27% <ø> (ø)
libraries/utils/kmeans.h 0.00% <ø> (ø)
libraries/utils/layoutloader.cpp 0.00% <0.00%> (ø)
libraries/utils/layoutmaker.cpp 0.79% <0.00%> (-0.68%) ⬇️
libraries/utils/mnemath.cpp 20.54% <0.00%> (-7.49%) ⬇️
libraries/utils/mnemath.h 0.00% <ø> (-16.00%) ⬇️
libraries/utils/spectral.cpp 13.25% <0.00%> (-1.82%) ⬇️
libraries/utils/spectral.h 0.00% <ø> (ø)
... and 286 more

@gabrielbmotta
Copy link
Collaborator Author

gabrielbmotta commented Jan 12, 2022

The qt installer action for linux is failing now working broken again.

@gabrielbmotta gabrielbmotta requested a review from juangpc January 14, 2022 19:15
@gabrielbmotta gabrielbmotta merged commit 0c3bc51 into mne-tools:main Feb 7, 2022
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