pymatgen icon indicating copy to clipboard operation
pymatgen copied to clipboard

Published 20 hours ago • verified publisher icon materialsproject

Chargemol autodownloads

Open chiang-yuan opened this issue 1 year ago • 3 comments

Try to close #3465

Checklist

  • [x] Google format doc strings added. Check with ruff.
  • [x] Type annotations included. Check with mypy.
  • [x] Tests added for new features/fixes.
  • [ ] ~~If applicable, new classes/functions/modules have duecredit @due.dcite decorators to reference relevant papers by DOI (example)~~

Tip: Install pre-commit hooks to auto-check types and linting before every commit:

pip install -U pre-commit
pre-commit install

chiang-yuan avatar Nov 15 '23 04:11 chiang-yuan

@janosh I use monkeypatch to mock the class method directly (rather than through module namespace) and add fake_path to atomic_densities_path to force the mock download function to be called. I think this work minimally already. Let me know what you think.

https://github.com/materialsproject/pymatgen/blob/65eb4fe1a59a23bbbbc2ccfe931c75ef29f64905/tests/command_line/test_chargemol_caller.py#L140

chiang-yuan avatar Jan 04 '24 12:01 chiang-yuan

@chiang-yuan I would like to merge this, but no tests are being run for some reason. Only pre-commit was run. Pls make sure the test suite is run properly. Thanks.

shyuep avatar Jun 14 '24 16:06 shyuep

@shyuep sorry this is a bit old and I am not sure why we need to test fake download as that does not really test if we download the file but mocks it offline. I could take care of autodownloads with #3778 together and the test there will really download chargmol during CI workflow

chiang-yuan avatar Jun 14 '24 19:06 chiang-yuan