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

Starting docs #95

Merged
merged 6 commits into from
May 10, 2016
Merged

Starting docs #95

merged 6 commits into from
May 10, 2016

Conversation

jakirkham
Copy link
Member

@jakirkham jakirkham commented Apr 15, 2016

Fixes #91

This is the start of a docs directory. I copied this ( conda-forge/staged-recipes#139 ) verbatim, but am definitely planning to change this a bit and possibly break out across several docs.

Also, was planning to add a few examples of repeated patterns in recipes that don't quite fit in any example yet. This way as we review we can add links to these for people that need a particular pattern. The hope is this sort of thing will reduce reviewer burden and demand less of existing reviewers. It will also hopefully standardizing things a bit more.

Eventually, I would like to have some sort of diagram to explain the automated workflow for feedstock generation. The hope is this will help organizations and individuals know what is going on. Also, it should help others if they need to handle some emergency. For those without access (permissions), they can at least identify a current issue and provide an issue with a little more information so someone with access can fix the problem a little faster.

@@ -0,0 +1,9 @@
This issue intends to layout some guidelines on the transfer of [conda-recipes]( https://github.com/conda/conda-recipes ) and [anaconda-recipes]( https://github.com/ContinuumIO/anaconda-recipes ) to conda-forge. These aren't hard and fast rules. They are certainly up for discussion. However, it would be good to come up with a consensus going forward.

Eventually all recipes from those repos should be proposed for addition here. It may be decided that a few don't actually belong or should not be supported anymore. For instance, a `vagrant` package seems a bit silly to me.
Copy link
Member

Choose a reason for hiding this comment

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

For instance, a vagrant package seems a bit silly to me.

I guess if we are starting to turn this into docs, we should probably drop this sentence.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I see this as two sections almost. One is the proposal of how we transfer Continuum recipes and interact with the communities that have formed around them. The other is more technical details that could almost be a spec that motivates (or has motivated) the linter and reviews in general.

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed.

@pelson
Copy link
Member

pelson commented Apr 15, 2016

Looks sensible to me. Merge when you are happy @jakirkham!

@jakirkham
Copy link
Member Author

Thanks @pelson. I think I'll just spend some time today working on this, but may try to do it after I work with @ocefpaf on netcdf as it feels that has been backlogged too long and it is holding back (or causing problems) for many other things.

@jakirkham
Copy link
Member Author

jakirkham commented May 5, 2016

Cleaned up indentation so it is not a bunch of really long lines.

@jakirkham jakirkham changed the title WIP*2: Starting docs WIP*2: Starting docs [ci skip] May 5, 2016
@jakirkham jakirkham changed the title WIP*2: Starting docs [ci skip] WIP*2: Starting docs May 5, 2016
@jakirkham
Copy link
Member Author

Could @pelson and @ocefpaf please read through this again and see if this sounds ok? I'm happy to take any revisions. Would be nice to do at least one more pass through before merging.

@jakirkham jakirkham changed the title WIP*2: Starting docs WIP: Starting docs May 5, 2016
recipes (if any) they would like to be notified for. If they are no longer
interested in any conda recipes, make that note here as well. Before contacting
anyone please consult the list below to see if that contributor has
restrictions.
Copy link
Member

Choose a reason for hiding this comment

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

This entire paragraph seems like a duplicate of the two paragraphs before this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops. That probably happened when I was trying to line wrap things. Will take a look. Thanks @ericdill.

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed. Doesn't look like there was anything suppose to be in its place.

@ericdill
Copy link
Member

ericdill commented May 5, 2016

Only one comment about a duplicate paragraph. Otherwise LGTM. Thanks for writing up these guidelines @jakirkham !

@ericdill
Copy link
Member

ericdill commented May 5, 2016

Ok I actually have another question... 🚲 🏠 (bike shed)

It seems like it might be worthwhile to give more guidance regarding exactly how to test conda packages. Would a reasonable option be to just point at some exemplar conda packages that test the various kinds of binaries that can be built with conda? i.e., an R package, a native library and a Python package (maybe even worth expanding python into three libraries: one that uses unittest, one with nose and one with pytest?)

@ChrisBarker-NOAA
Copy link
Contributor

Actually, I have no idea HOW to th conda packages in a universal way (even if we restrict ourselves to python). For python packages:

You can (and should) at least test that the pacakge can be imported -- that's easy and should be in the docs for how to build a "good" conda package. But to run "real" tests:

There are essentially two ways to to manage unit tests in python package -- put them in the package itself, or keep them separate.

  • If they are in inside the package, then you should be able to run something like:

python -c "import the_package; the_package.run_tests()"

but AFAIK, there is no standard for the test command, etc, so you have to do it custom for each package (and they each may require different test runners, etc)

  • If they are outside the pacakge, then ideally the package's setup.py has been set up so you can do:

setup.py test

if not, then you need to do something like:

cd tests; py.test
or
cd tests: nosetests

And the details will be package specific -- again, no real standard.

However, here's the rub -- this works in plain old install mode:

pip install .
python setup.py test
or
py.test

or....

BUT: if the tests are outside the package, then they do not get installed with pip install, and then they don't get bundled up into the conda package, so there is no way to access them with a simple conda test mode -- the test code isn't there.

I haven't figured out a good way to deal with this yet -- it seems you would have to write a test script that downloads and unpacks the source inside the test environment, and then goes in and runs the tests. but that would be a pain and platform dependent, and...

Maybe we need to suggest an enhancement to conda that makes it easy to do this in a test script?

Note: I started a thread on this on the conda list, but never really got to a satisfying solution:

https://groups.google.com/a/continuum.io/forum/#!searchin/conda/How$20to$20run$20tests$20that$20aren$27t$20in$20the$20package$3F/conda/rTq72-jsJf8/4lb3u0dbCwAJ

OK -- this got pretty off-topic, maybe we need to start a new issue....

@jakirkham
Copy link
Member Author

Ok I actually have another question... 🚲 🏠 (bike shed)

What is your favorite color? 😆

It seems like it might be worthwhile to give more guidance regarding exactly how to test conda packages. Would a reasonable option be to just point at some exemplar conda packages that test the various kinds of binaries that can be built with conda? i.e., an R package, a native library and a Python package (maybe even worth expanding python into three libraries: one that uses unittest, one with nose and one with pytest?)

100% agree. I'm trying to decide though whether that info belongs here or in PR template ( conda-forge/staged-recipes#550 ) and/or the addition of some more examples at staged-recipes ( conda-forge/staged-recipes#318 ). I'm not sure how nitty-gritty we want to get here. Kind of aiming for this to be an overview of how to generally proceed and what sorts of things are important.

To respond to some of your more specific points (regardless of where this lives), I have replied to them below.

...an R package...

Currently we don't have these and personally I have not written one. So, am not quite sure what this will entail until we get into it more. Related questions include Perl and Lua, which again I have not tried.

...a native library...

👍 We were thinking CMake and make probably as primary examples.

...a Python package (maybe even worth expanding python into three libraries: one that uses unittest, one with nose and one with pytest?)...

I'm ok with the Python package idea. Am much less concerned with the types of test suites. In practice, we run these on a case-by-case. Maybe we should just always run them. That is up for discussion.

There are several reasons we don't always do this. One is we normally assume pure Python (especially if tested with CI) is probably released clean and thus doing this is a bit unnecessary. Another is also the concern that @ChrisBarker-NOAA raises above (thanks for writing that so I didn't have to 😉) about how to do it being varied. Also, there is also the question of whether tests are even in the release (this turns into a "where do we get the source from"). Though there can be complication that go beyond this list.

Generally, we have tried to run tests if the Python code includes any compilation (Cython, C, C++, etc.) though. Still this involves solving it in a case-by-case basis.

@ericdill
Copy link
Member

ericdill commented May 5, 2016

Actually, I have no idea HOW to th conda packages in a universal way

I agree that this an intractable (or nearly so) problem to solve in a universal way. I was really thinking that we could just add some guidance that would help in most (or at least some) of the conda packages that contributors would provide.

You can (and should) at least test that the pacakge can be imported -- that's easy and should be in the docs for how to build a "good" conda package. But to run "real" tests:
There are essentially two ways to to manage unit tests in python package -- put them in the package itself, or keep them separate.
If they are in inside the package, then you should be able to run something like:
python -c "import the_package; the_package.run_tests()"
but AFAIK, there is no standard for the test command, etc, so you have to do it custom for each package (and they each may require different test runners, etc)
If they are outside the pacakge, then ideally the package's setup.py has been set up so you can do:
setup.py test
if not, then you need to do something like:
cd tests; py.test
or
cd tests: nosetests
And the details will be package specific -- again, no real standard.
However, here's the rub -- this works in plain old install mode:
pip install .
python setup.py test
or
py.test
or....
BUT: if the tests are outside the package, then they do not get installed with pip install, and then they don't get bundled up into the conda package, so there is no way to access them with a simple conda test mode -- the test code isn't there.
I haven't figured out a good way to deal with this yet -- it seems you would have to write a test script that downloads and unpacks the source inside the test environment, and then goes in and runs the tests. but that would be a pain and platform dependent, and...

AFAICT this would be a really good starting point for adding some guidance in terms of how to manage test(s) for the test section of the meta.yaml.

Am much less concerned with the types of test suites.

The only reason I suggested that we might want to include some guidance regarding the different types of test suites is that (I think?) the test suites have different auto-discovery mechanisms and it is not necessarily obvious how to make conda find the test files that are part of the conda package (if they are even shipped with the conda package!). That being said, I'm pretty sure I've sufficiently derailed this issue and we can save this discussion for a different issue.

@jakirkham
Copy link
Member Author

I totally agree that we can improve on the documentation here, @ericdill. It's just I realized that my desire for perfection was getting in the way of completion. Not to mention, there is so much to do here and in my other work outside of this that this rough but basically working copy has gotten terribly backlogged.

Instead of trying to make this perfect, we should really embrace this as a living document. The first step is to get out the door I think. From there, I am more than happy to iterate with you and others on figuring out where various examples and details live throughout conda-forge.

@ericdill
Copy link
Member

Instead of trying to make this perfect, we should really embrace this as a living document. The first step is to get out the door.

👍 let's get this in and improve it iteratively. Thanks for the work on this and sorry I derailed getting this merged

@jakirkham
Copy link
Member Author

Nothing to be sorry about.

@jakirkham jakirkham changed the title WIP: Starting docs Starting docs May 10, 2016
@jakirkham
Copy link
Member Author

Looks sensible to me. Merge when you are happy @jakirkham!

Alright, @pelson, I will finally take you up on your offer. Added a few changes that I think should make it a bit better. Though there is still quite a bit of room for growth here and welcome everyone to participate.

@jakirkham jakirkham merged commit 47e7533 into conda-forge:master May 10, 2016
@jakirkham jakirkham deleted the add_docs branch May 10, 2016 01:48
@jakirkham jakirkham mentioned this pull request May 10, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Docs only repo
4 participants