Codecov Report

Merging #115 (9fabe2a) into trunk (0a158de) will decrease coverage by 0.21%.
The diff coverage is 93.15%.

@@            Coverage Diff             @@
##            trunk     #115      +/-   ##
==========================================
- Coverage   96.35%   96.14%   -0.21%     
==========================================
  Files          10       10              
  Lines        1069     1142      +73     
==========================================
+ Hits         1030     1098      +68     
- Misses         39       44       +5     

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Sorry for the late reply, finally have some time to add a pytest.

Unfortunately, I cannot generate a tb.dat file that is fully consistent with the current hr.dat file in the pytest regression data, because

1. the Hamiltonian depends on the gauge (from Wannierization)
2. and most importantly, it seems the current W90 v3.1 generates a different set of R vectors (use_ws_distance = .true.) than the current wsvec.dat in regression data, see the new file here
   

   TBmodels/tests/samples/silicontb_wsvec.dat

   Lines 139 to 141 in 130bf4f

   	-3 1 1 6 1
   	1
   	4 0 -4

   
   and the old file
   

   TBmodels/tests/samples/silicon_wsvec.dat

   Lines 139 to 142 in 130bf4f

   	-3 1 1 6 1
   	2
   	0 0 0
   	4 0 -4

So in the end I added new regression data for the tb.dat pytest, instead of comparing it with the existing regression data for hr dat.

Also, it seems the pre-commit has some version incompatibilities between black and click, and pylint. Not sure what are the versions you were using?

If we generate a hr.dat, tb.dat and wsvec from the same Wannier90 run, can we add a test that compares the two method for loading the data?
IMO we could also replace the previous regression data, since the load from hr.dat hasn't changed in this PR.

The pre-commit issue is due to psf/black#2964; the easiest change is changing the click requirement to click>=7.0, !=7.1.0, <8.1 for the time being; I will update the pre-commit dependencies (and probably switch to poetry so we get consistent CI runs) in a separate PR.
The pre-commit is fairly outdated though, and FMPOV it's fine if we just ignore it for this PR. I can upgrade and fix it in a separate PR.

If we generate a hr.dat, tb.dat and wsvec from the same Wannier90 run, can we add a test that compares the two method for loading the data? IMO we could also replace the previous regression data, since the load from hr.dat hasn't changed in this PR.

Yes this is the best solution!

The pre-commit issue is due to psf/black#2964; the easiest change is changing the click requirement to click>=7.0, !=7.1.0, <8.1 for the time being; I will update the pre-commit dependencies (and probably switch to poetry so we get consistent CI runs) in a separate PR. The pre-commit is fairly outdated though, and FMPOV it's fine if we just ignore it for this PR. I can upgrade and fix it in a separate PR.

That would be great!

So, to proceed, maybe you could first update the pre-commit and poetry stuff, then I will make sure this PR doesn't break any of these checks. For the tb.dat file, either you regenerate the test data (tb.dat, and the hdf5 test files...), or I can regenerate and replace the old files, which one do you prefer?

So, to proceed, maybe you could first update the pre-commit and poetry stuff

This is done, but produces a merge conflict because I switched to a src/ layout. Let me know if I should help out in rebasing / merging that.

can regenerate and replace the old files

If you could regenerate them with the latest Wannier90 would be great, thanks!