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
Add Deprecation Warning for Namespace Change #805
Conversation
SimonBoothroyd
commented
Jan 6, 2021
- Tag issue being addressed (Migrate openforcefield to openff.toolkit #804)
- Add tests
- Update docstrings/documentation, if applicable
- Lint codebase
- Update changelog
Co-authored-by: Jaime Rodríguez-Guerra <jaimergp@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Diff looks good to me with one note. I grabbed this branch locally and it worked as expected
warnings.warn( | ||
"Importing this package as `import openforcefield.XXX` and " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non-blocking, but it would be nice to have more precise language here
- The entire package is never really imported
- Saying "importing this module as ... " is incorrect when only a class is imported
- Likewise, "importing this class/etc. ..." is also incorrect when entire modules are imported
Maybe something more general like "Importing components from this package as ... " / "From version 0.9.0
onwards, components of this package will need to be imported as ..." ?
Co-authored-by: Matt Thompson <mattwthompson@protonmail.com>
Co-authored-by: Matt Thompson <mattwthompson@protonmail.com>