RF: Write DFT database manager as object

[effigies]

Using pytest-xdist, it turns out we have some racing tests in test_dft.py.

This adds a dft._DB class that handles the _init_db and db(no)change functions. The default instance remains at dft.DB, but this allows us to create new instances for testing purposes. We then create per-test instances in-memory to avoid any chance at races and monkey-patch them in to the dft module.

This also updates the database connection to allow sqlite's own locking mechanisms to ensure consistency. While in tests before settling on the monkeypatch approach this seemed to reduce the frequency of races significantly, I was unable to eliminate them entirely. This module should not be considered multiprocessing-safe.

[codecov]

Codecov Report

Base: 92.30% // Head: 92.40% // Increases project coverage by +0.10% 🎉

Coverage data is based on head (32bc89a) compared to base (8cf190d).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1158      +/-   ##
==========================================
+ Coverage   92.30%   92.40%   +0.10%     
==========================================
  Files          98       98              
  Lines       12236    12248      +12     
  Branches     2515     2516       +1     
==========================================
+ Hits        11294    11318      +24     
+ Misses        618      611       -7     
+ Partials      324      319       -5     

Impacted Files	Coverage Δ
nibabel/dft.py	86.06% <100.00%> (+4.29%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[effigies]

@chaselgrove @matthew-brett Any chance of a review? Making databases multiprocessing safe is not something I've done a lot of.

[chaselgrove]

@chaselgrove @matthew-brett Any chance of a review? Making databases multiprocessing safe is not something I've done a lot of.

@effigies I can't say I ever have! So if pytest-xdist is happy, then that's a win. The changes as a whole look fine to me. There is the side effect that the database file is created when dft is imported (rather than when the database is accessed, as before), but I won't lose sleep over it.

[effigies]

The database was always initialized at import, but I wouldn't be opposed to shifting it to first access.

[effigies]

@chaselgrove Updated to create the database on access, but while testing I found that I was not able to eliminate races entirely with everything I did. I've updated the description with the new approach.

[effigies]

I'm going to merge to get tests passing again. Happy to make some post-merge adjustments if y'all see any.