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 a bug in pwscf.py. The proc_val function modifies string values. #3172

Merged
merged 5 commits into from
Jul 22, 2023

Conversation

pablogalaviz
Copy link
Contributor

@pablogalaviz pablogalaviz commented Jul 21, 2023

Summary

The proc_val function modifies the value of string variables (changes 'd' for 'e'). For example, smearing = 'cold' becomes smearing = 'cole'. It is solved by using a local variable.

Major changes:

Added a local variable

    val_replace = val.replace("d", "e")
    return smart_int_or_float(val_replace)

Todos

Checklist

  • Google format doc strings added. Check with ruff.
  • Type annotations included. Check with mypy.
  • 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

…ring variables (changes 'd' for 'e'). For example, smearing = 'cold' becomes smearing = 'cole'. Solved by using a local variable.
Copy link
Member

@janosh janosh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @pablogalaviz! We need a new test for this that fails without this fix.

@janosh janosh added needs testing PRs that are not ready to merge due to lacking tests io Input/output functionality fix Bug fix PRs labels Jul 21, 2023
@janosh
Copy link
Member

janosh commented Jul 22, 2023

Nice test, thanks @pablogalaviz! 👍

@janosh janosh enabled auto-merge (squash) July 22, 2023 14:02
@janosh janosh merged commit 38c8928 into materialsproject:master Jul 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Bug fix PRs io Input/output functionality needs testing PRs that are not ready to merge due to lacking tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants