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

[FEA]: Add check to CI to detect incorrect imports of morpheus._lib modules #727

Closed
2 tasks done
mdemoret-nv opened this issue Feb 21, 2023 · 0 comments · Fixed by #1112
Closed
2 tasks done

[FEA]: Add check to CI to detect incorrect imports of morpheus._lib modules #727

mdemoret-nv opened this issue Feb 21, 2023 · 0 comments · Fixed by #1112
Assignees
Labels
feature request New feature or request

Comments

@mdemoret-nv
Copy link
Contributor

Is this a new feature, an improvement, or a change to existing functionality?

New Feature

How would you describe the priority of this feature request

High

Please provide a clear description of problem this feature solves

Imports of morpheus._lib should be used very rarely and will follow a specific format when used correctly. Sometimes tools like VS Code can auto add imports to the morpheus._lib module when a similar name is used in morpheus. For example, from morpheus._lib.messages import MessageMeta should really be from morpheus.messages import MessageMeta which will auto select the correct class depending on whether C++ is enabled or not.

Describe your ideal solution

Since we should never be using from morpheus._lib.XXX import YYY it should be easy to detect in CI any incorrect uses. The following is a list of the rules:

  • Ban: from morpheus._lib.XXX import YYY
    • Instead, it should be from morpheus.XXX import YYY to pick the correct C++/Python version
    • This may require re-exporting the C++ symbols in an __init__.py
  • Ban: import morpheus._lib.XXX
    • Instead, it should be import morpheus.XXX
  • Ban: import morpheus._lib.XXX as YYY
    • Instead, it should be import morpheus.XXX as YYY
  • Allow: import morpheus._lib.XXX as _XXX
    • This is the only way that C++ modules should be imported.
    • It should be redefined as a new name to prevent accidental re-export
    • Most of the stages follow this format already

Describe any alternatives you have considered

Looked for ways to prevent the auto importer in VSCode from using morpheus._lib, but it doesnt seem possible.

Additional context

This recently broke the SID Visualization example in 23.01.00

Code of Conduct

  • I agree to follow this project's Code of Conduct
  • I have searched the open feature requests and have found no duplicates for this feature request
@mdemoret-nv mdemoret-nv added the feature request New feature or request label Feb 21, 2023
@pdmack pdmack changed the title [FEA]: Add check to CI to detect incorrect imports of morpheus._lib moduels [FEA]: Add check to CI to detect incorrect imports of morpheus._lib modules Feb 28, 2023
@dagardner-nv dagardner-nv self-assigned this Jul 31, 2023
rapids-bot bot pushed a commit that referenced this issue Aug 17, 2023
* Publish artifact urls in job summaries fixes #1105
* Remove redundant apt installs for packages that exist in the conda env fixes #1106
* Checks now performs a build first. This allows pylint to accurately warn for undefined members. fixes #956
* Custom pylint checker to validate correct importing of `morpheus._lib` fixes #727

Authors:
  - David Gardner (https://github.com/dagardner-nv)

Approvers:
  - Michael Demoret (https://github.com/mdemoret-nv)

URL: #1112
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants