Fix regression when deps=False is set#187
Conversation
agriyakhetarpal
left a comment
agriyakhetarpal
left a comment
There was a problem hiding this comment.
Thanks, @ryanking13! I have one question about the code, which is just for my understanding and won't block merging.
| try: | ||
| # pyodide-micropip-test depends on snowballstemmer | ||
| import snowballstemmer # noqa: F401 | ||
| except ModuleNotFoundError: | ||
| pass | ||
| else: | ||
| raise Exception("Should raise!") |
There was a problem hiding this comment.
As the intention here is that we want import snowballstemmer to not be installed due to deps=False, is there a better way to handle this?
i.e., is it possible to wrap it in a context manager with with pytest.raises(ModuleNotFoundError (or we have to do it in the current way because pytest isn't installed?)?
There was a problem hiding this comment.
i.e., is it possible to wrap it in a context manager with with pytest.raises(ModuleNotFoundError (or we have to do it in the current way because pytest isn't installed?)?
Yeah, good point. pytest.raises would work too, but I wanted to keep the test simple, not loading any extra package.
|
Thanks for the review! |
2 participants
resolves #180
I forgot to call
awaitwhen deps is not set.