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: import fails due to pytest ModuleNotFoundError #78

Merged
merged 3 commits into from Oct 27, 2023

Conversation

simmsa
Copy link

@simmsa simmsa commented Oct 27, 2023

import pecos currently fails due to pytest being called on this line:

@pytest.mark.skip()

This commit moves pytest up to the required section of requirements.txt which allows pytest to be installed during the pip installation step.

`import pecos` currently fails due to `pytest` being called on this line: https://github.com/sandialabs/pecos/blob/e0aea99535f92f6f63f38387a79898ccb6966f37/pecos/graphics.py#L423

This commit moves pytest up to the required section which allows pytest to be installed during the `pip` installation step.
@kaklise
Copy link
Member

kaklise commented Oct 27, 2023

@simmsa Thanks for bringing this to my attention! requirements.txt is only used by the workflows and all packages are installed regardless of order. This is why the tests passed (at least they used to pass, looks like some changes are needed for the most recent version of pandas) . For this to install properly using pip, I think I need to add pytest to the setup.py file.

@simmsa
Copy link
Author

simmsa commented Oct 27, 2023

@kaklise thanks for looking into this! It seems like this pull request doesn't actually change anything. For whatever reason I thought that the comments in the requirement.txt file were directives. It sounds like you have a good idea for how to fix this issue, but I would be happy to help. I am installing pecos via pip on the latest anaconda on Windows 10.

@simmsa
Copy link
Author

simmsa commented Oct 27, 2023

@kaklise do you think adding pytest to this array will fix this issue?

Require installation of `pytest` on pip installation of pecos. Should fix: sandialabs#79
@kaklise
Copy link
Member

kaklise commented Oct 27, 2023

@simmsa Yes, I just tested that out on a clean environment and that resolved the import error after installation. I'll make that change, and I'll merge your PR since pytest is now a required package and should be listed that way in requirements.txt.

@kaklise
Copy link
Member

kaklise commented Oct 27, 2023

@simmsa just saw that you updated the file. Thanks!

@simmsa
Copy link
Author

simmsa commented Oct 27, 2023

@kaklise thank you for verifying it works!

setup.py Outdated
@@ -17,7 +17,8 @@
'install_requires': ['numpy >= 1.10.4',
'pandas >= 0.18.0',
'matplotlib',
'jinja2'],
'jinja2'
Copy link
Member

Choose a reason for hiding this comment

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

missing a comma between jinja2 and pytest

@kaklise kaklise merged commit f176b1d into sandialabs:main Oct 27, 2023
31 checks passed
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