pythonpackage can't return build requirements for wheels, make it return an error if attempted #1852
Conversation
|
This bug mainly manifests itself in that unit test by the way, not any situation I can think of when actually using the |
Make sure the pythonpackage functions return an error when someone attempts to do so anyway
ghost
changed the title
| "python-for-android", include_build_requirements=True, | ||
| verbose=True, | ||
| ) | ||
| except NotImplementedError as e: |
There was a problem hiding this comment.
I'd be tempted to make an IsWheelError subclass or something, on the basis that specific exceptions aren't used often enough, but I'm never really sure about this.
There was a problem hiding this comment.
@inclement I'm somehow afraid somebody will use that to test if a package is a wheel and then end up with broken code once it is actually implemented and this no longer returns an error (or they use it right but we remove the error so it can no longer be imported once this is implemented and therefore no longer ever thrown, and that breaks their code). So I think I'd rather stick with a NotImplementedError here, personally
There was a problem hiding this comment.
@inclement what are your thoughts on my thought on this? 😛 other than this I think it's ready for merge
There was a problem hiding this comment.
I'm merging, we can always eventually address that one later if we feel it's critical
There was a problem hiding this comment.
I wouldn't have approved the PR if I thought this needed changing, sorry if that wasn't clear and thanks for explaining the motivation!
ghost
mentioned this pull request
opacam
added
the
Priority: Critical
|
Thanks @AndreMiras!!, |
|
I agree we should always keep the build green |
This change ensures the pythonpackage functions return an error when someone attempts to get build requirements of a wheel (beyond the regular runtime dependencies) since this is currently not implemented properly.
It might be possible to determine and obtain this info somehow, but since this is not ever actually used by our
setup.pyfunctionality I have made sure it raises aNotImplementedErrornow if this is ever attempted, instead of returning incorrect data as previously.This should also fix the test failure of
test_pythonpackage_basic.pyon current latestdevelop.