-
-
Notifications
You must be signed in to change notification settings - Fork 90
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
Problem in version 2.0.0 #699
Comments
Thank you for the report. I agree that we shouldn't have test-dependencies in the production code. |
Yeah, it turns out this is why all of our doc builds are breaking too. I can fix it in about 2 hours.... |
mwcraig
added a commit
to mwcraig/ccdproc
that referenced
this issue
Sep 5, 2019
mwcraig
added a commit
that referenced
this issue
Sep 5, 2019
…owards-correct-code-but-i-have-my-doubts Actually fix #699 by removing the offending import line
mwcraig
added a commit
that referenced
this issue
Sep 6, 2019
…owards-correct-code-but-i-have-my-doubts Actually fix #699 by removing the offending import line
@simontorres -- 2.0.1 is on pypi and fixes this issue. Thanks for pointing it out! |
Awesome! Thanks @mwcraig |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Summary
In my opinion a production package do not need to install all development tools. Or from another angle in test code you import from your source file but never in the other direction and this is happening in
image_collection.py
References here are from master branch
ccdproc/ccdproc/image_collection.py
Line 21 in 14c2218
And
ccdproc/ccdproc/image_collection.py
Lines 1006 to 1014 in 14c2218
The problem here is that this will require pytest. We are working with containerized applications were you want to be as minimalist as possible.
Another option is that
pytest
should be in the included as a dependency. (installed automatically upon callingpip install ccdproc==2.0.0
. However I would like to insist that importing fromtest
is wrong.My
install_requires
variable contains the following.The text was updated successfully, but these errors were encountered: