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

BUG: better handling the optional dependency required for functionality #3039

Merged
merged 2 commits into from
Jun 14, 2024

Conversation

bsipocz
Copy link
Member

@bsipocz bsipocz commented Jun 14, 2024

One of the remote tests were failing when regions wasn't installed, which highlighted the case that we didn't handle the missing dependency well in the function too (e.g. the module should not fail if it's missing, but it should fail rather than just print a message when someone tries to use the method).

@bsipocz bsipocz added this to the v0.4.8 milestone Jun 14, 2024
@bsipocz bsipocz requested a review from andamian June 14, 2024 21:17
@@ -218,6 +218,7 @@ def get_enhanced_table(result):
print(
"Could not import astropy-regions, which is a requirement for get_enhanced_table function in alma."
"Please refer to http://astropy-regions.readthedocs.io/en/latest/installation.html for how to install it.")
raise

Choose a reason for hiding this comment

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

Hmm. I thought I did this. Wondering if that got lost in the rebase exercise which gave me a bit of a headache...

@andamian
Copy link

Sorry about this. Pretty sure I had it right but it probably got lost when I did the rebasing.

@bsipocz
Copy link
Member Author

bsipocz commented Jun 14, 2024

No worries, it should have been picked up by the review anyway, but I only noticed it as I saw the test failing today in an env where regions was not present.

@bsipocz bsipocz merged commit d7aba96 into astropy:main Jun 14, 2024
8 checks passed
@bsipocz bsipocz deleted the BUG_handling_optional_regions branch June 18, 2024 06:30
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.

2 participants