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

ENH: warn user in case a Dataset subclass is redefined #3495

Merged
merged 1 commit into from
Sep 17, 2021

Conversation

neutrinoceros
Copy link
Member

@neutrinoceros neutrinoceros commented Aug 31, 2021

PR Summary

Following a recent discussion on the Slack space in which we discussed how yt extensions
provided support to create third party frontends, which allows users to extend over native one as

import yt
import yt.extensions.my_frontend

The only issue I was able to see with this workflow is that, in case such a frontend gets integrated
into yt itself, then users may not realize that the extension import became useless, or worse. Shadowing a "first citizen" frontend may actually be useful as temporary measure, which is why I'm not making this an error, but it seems fair to emit a warning as a forgiving gesture to transiting users.

PR Checklist

[N/A] New features are documented, with docstrings and narrative docs

  • Adds a test for any bugs fixed. Adds tests for new features.

note : I want to add documentation for the whole extension frontend workflow later, but I think this small patch counts as its own documentation.

@neutrinoceros neutrinoceros added enhancement Making something better UX user-experience labels Aug 31, 2021
@brittonsmith
Copy link
Member

This seems like a reasonable solution to me. I wouldn't want to prevent an extension over-writing an internal frontend, but a warning to make it clear what's happening is good.

@neutrinoceros
Copy link
Member Author

@Xarthisius @brittonsmith, any reason not to merge this ?

@Xarthisius Xarthisius merged commit 0623cdf into yt-project:main Sep 17, 2021
@neutrinoceros neutrinoceros deleted the warn_redef_frontend branch September 18, 2021 05:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Making something better UX user-experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants