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

Predict an import error with jaraco.vcs.fixtures #39

Closed
wants to merge 1 commit into from

Conversation

bswck
Copy link
Contributor

@bswck bswck commented Aug 6, 2024

No description provided.

@jaraco
Copy link
Owner

jaraco commented Aug 6, 2024

I'm kinda disinclined to add the conditional import. I expect no one to import jaraco.vcs.fixtures unless they have pytest installed (and have the fixture enabled).

Is there a scenario where this would need to be supported?

@bswck
Copy link
Contributor Author

bswck commented Aug 6, 2024

I'm kinda disinclined to add the conditional import. I expect no one to import jaraco.vcs.fixtures unless they have pytest installed (and have the fixture enabled).

Is there a scenario where this would need to be supported?

This isn't really a conditional import (we're reraising the exception), but a guide on what to do just in case of an error.
For instance, one may be testing with unittest only and think that jaraco.vcs.fixtures has something for them.

@jaraco
Copy link
Owner

jaraco commented Aug 7, 2024

Right. I see that now. I do try to keep the code simple and tight and only add behavior where it has demonstrable value. Here the value seems small if not negligible. Making this change also implies this change should be applied to all other fixtures in the ecosystem. Let's instead rely on the benefits of inversion of control and dependency injection that entry points provides and assume that pytest will be present. If anyone encounters this error and is confused by it, they can report their use case and we can consider an intervention then.

@jaraco jaraco closed this Aug 7, 2024
@bswck bswck deleted the expect-import-error branch August 7, 2024 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants