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 missing path debug #395

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix missing path debug #395

wants to merge 2 commits into from

Conversation

amarkpayne
Copy link
Member

I caught a typo in the messages the Arkane adapter returns when file paths are missing. While I was at it, I figured I would quickly refactor this code so that all missing file paths could be displayed if needed.

If you don't like the refactoring, I am fine with excluding all but the typo fix.

@codecov
Copy link

codecov bot commented May 12, 2020

Codecov Report

Merging #395 into master will not change coverage.
The diff coverage is 11.11%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #395   +/-   ##
=======================================
  Coverage   54.54%   54.54%           
=======================================
  Files          34       34           
  Lines       11904    11904           
  Branches     3633     3633           
=======================================
  Hits         6493     6493           
  Misses       4458     4458           
  Partials      953      953           
Impacted Files Coverage Δ
arc/statmech/arkane.py 67.87% <11.11%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2e71a1a...41b109c. Read the comment docs.

Copy link
Contributor

@xiaoruiDong xiaoruiDong left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! The change makes sense and increases the robustness. I only had a minor comments:
At this line

if freq_path:

which is in the same function, it will determine freq_path validation again, which I think is no longer needed. Can you take a look, and see if we can remove that if block? Thanks!

@alongd
Copy link
Member

alongd commented Jun 7, 2020

@amarkpayne, what is the status of this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants