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

Fix mock_run_vasp to accept args and test docs #151

Merged
merged 3 commits into from
Jul 4, 2022

Conversation

mjwen
Copy link
Member

@mjwen mjwen commented Jul 2, 2022

This PR introduces two fixes related to the test system.

  • Fix mock_run_vasp to accept *args and **kwargs to match the signature of run_vasp. Otherwise, if a job maker sets some of these arguments (for example, the coming MD maker will set handler), the test will fail.

  • Fix doc on writing tests: changing the unit cell of the Si structure to avoid failing check_poscar. In check_poscar, the user poscar will use a lattice of

    3.348898 0.000000 1.933487
    1.116299 3.157372 1.933487
    0.000000 0.000000 3.866975
    

    while the ref poscar will use whatever lattice provided in the test file. The current doc on test suggests to use

0.000000 2.730000 2.730000
2.730000 0.000000 2.730000
2.730000 2.730000 0.000000

A test will fail if the two do not match. Currently, I just update the doc to make the the same. But I believe there should be better ways to do it. For example, what if a new maker do not want to use Si as the test system? Basically, it seems the user poscar is hard-coded? I know it is copied from somewhere to a tmpdir (cwd), but I don't know where the copy happens.

Additional dependencies introduced (if any)

None

TODO (if any)

None

Checklist

Work-in-progress pull requests are encouraged, but please put [WIP] in the pull request
title.

Before a pull request can be merged, the following items must be checked:

  • Code is in the standard Python style.
    The easiest way to handle this is to run the following in the correct sequence on
    your local machine. Start with running black on your new code. This will
    automatically reformat your code to PEP8 conventions and removes most issues. Then run
    pycodestyle, followed by flake8.
  • [NA] Docstrings have been added in theNumpy docstring format.
    Run pydocstyle on your code.
  • [NA] Type annotations are highly encouraged. Run mypy to
    type check your code.
  • [NA] Tests have been added for any new functionality or bug fixes.
  • All linting and tests pass.

@codecov
Copy link

codecov bot commented Jul 2, 2022

Codecov Report

Merging #151 (9be35b1) into main (f238e11) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #151   +/-   ##
=======================================
  Coverage   73.14%   73.14%           
=======================================
  Files          49       49           
  Lines        4253     4253           
  Branches      672      672           
=======================================
  Hits         3111     3111           
  Misses        973      973           
  Partials      169      169           

@utf
Copy link
Member

utf commented Jul 4, 2022

Thanks!

@utf utf merged commit fe427ae into materialsproject:main Jul 4, 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.

None yet

2 participants