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

Make Travis run doctests. #405

Closed
wants to merge 3 commits into from
Closed

Conversation

ketch
Copy link
Member

@ketch ketch commented May 26, 2014

I set up a bunch of doctests long ago, but Travis wasn't running them. Now it will.

Since nose imports files when looking for tests, I had to modify cleanup_examples.py so that it wouldn't actually do anything on import.

I also fixed one test that had broken.

@ketch
Copy link
Member Author

ketch commented May 26, 2014

So apparently, in order to run doctests, nose has to import every module it finds. This is failing on Travis because some of our modules import things like pyweno, scipy, and petsc4py which are not available.

It also means that if you wrote a script that executed shell commands, you could trash your whole filesystem by running nosetests --with-doctest, which seems like a bad behavior but is apparently accepted.

I'm not sure what to do.

@sanromd
Copy link
Contributor

sanromd commented May 26, 2014

is there anyway to tell nose tests to try to load and if the module is not
present simply skip it with a warning? ​

Damián

ubi dubium ibi libertas

On 26 May 2014 13:29, David Ketcheson notifications@github.com wrote:

So apparently, in order to run doctests, nose has to import every module
it finds. This is failing on Travis because some of our modules import
things like pyweno, scipy, and petsc4py which are not available.

It also means that if you wrote a script that exectued shell commands, you
could trash your whole filesystem by running nosetests --with-doctest,
which seems like a bad behavior but is apparently accepted.

I'm not sure what to do.


Reply to this email directly or view it on GitHubhttps://github.com//pull/405#issuecomment-44176877
.

@ketch
Copy link
Member Author

ketch commented May 26, 2014

@sanromd No. The thing is, running those modules is not even part of nose's intended behavior.

@ahmadia
Copy link
Member

ahmadia commented May 27, 2014

I've had mixed experiences with doctests. I'm not using them in any of our other major projects. That said, I don't think it would be too hard to get all of the PyClaw dependencies installed on Travis so that the tests can run.

@ketch
Copy link
Member Author

ketch commented May 28, 2014

I love doctests! I view examples as the most useful part of documentation, and your examples will break if you don't have doctests.

Here are the dependencies we'll need:

  • pyweno
  • scipy
  • petsc4py
  • h5py

Alternatively, most of these imports could be moved inside functions and then we wouldn't need to install them on Travis. Imports inside functions make more sense to me, PEP 8 notwithstanding. Opinions?

@ahmadia
Copy link
Member

ahmadia commented May 28, 2014

That's not a hard stack to build...

On Wednesday, May 28, 2014, David Ketcheson notifications@github.com
wrote:

I love doctests! I view examples as the most useful part of documentation,
and your examples will break if you don't have doctests.

Here are the dependencies we'll need:

  • pyweno
  • scipy
  • petsc4py
  • h5py

Alternatively, most of these imports could be moved inside functions and
then we wouldn't need to install them on Travis. Imports inside functions
make more sense to me, PEP 8 notwithstanding. Opinions?


Reply to this email directly or view it on GitHubhttps://github.com//pull/405#issuecomment-44367030
.

@ketch
Copy link
Member Author

ketch commented May 28, 2014

Some questionable alternatives are here: http://stackoverflow.com/a/3653750/786902

And there's a prior issue I encountered: #409

@mandli
Copy link
Member

mandli commented Jul 12, 2014

@ahmadia mentioned that binary hashdist installs were close so let's plan on having these dependencies on travis and not modify the placement of the import statements.

@ahmadia
Copy link
Member

ahmadia commented Jul 12, 2014

We had a proof of concept thanks to @asmeurer yesterday. I think we can have something stable enough to use in Clawpack by the end of the month.

@ahmadia
Copy link
Member

ahmadia commented Dec 17, 2014

We had a proof of concept thanks to @asmeurer yesterday. I think we can have something stable enough to use in Clawpack by the end of the month.

Ooph. I have to revise this to January 2015, mostly because we haven't had dedicated developer time on HashDist to finish this.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling c441c59 on ketch:travis-doctests into 40a7249 on clawpack:master.

@ketch
Copy link
Member Author

ketch commented Dec 18, 2014

This is now passing, thanks to @mandli's help with command-line arguments to nose. We're omitting some things, but running the doctests in solver, geometry, solution, and controller is an improvement so I'd like to merge this.

@@ -1,32 +1,31 @@

Copy link
Member

Choose a reason for hiding this comment

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

Prefix the file with #!/usr/bin/env python and set permissions +x and it can be run directly as a script.

Copy link
Member

Choose a reason for hiding this comment

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

I would put this file in a subdirectory like scripts.

@ahmadia
Copy link
Member

ahmadia commented Dec 18, 2014

Looks good. +1

@ketch
Copy link
Member Author

ketch commented Dec 21, 2014

This is now superseded by #487, which is identical except that it doesn't delete codegen.py.

@ketch ketch closed this Dec 21, 2014
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

5 participants