- 
                Notifications
    
You must be signed in to change notification settings  - Fork 158
 
Use mlir-python-extras as a dependency #1828
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
Conversation
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
| 
           Waiting on makslevental/mlir-python-extras#101  | 
    
| 
           Should be ready to go now!  | 
    
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.
LGTM
| 
           I was going to wait to merge until #1835 was in, so this PR would be tested with that, but it's taking a while so I did the test locally with quick_setup and it worked for me, so I'll go ahead and merge this now.  | 
    
This PR removes code in
mlir-aie/python/extrasthat is currently present inmlir-python-extrasand usesmlir-python-extrasas a dependency. To differentiate more clearly between what is inmlir-python-extrasand what is added, I also separated the import paths; theextrasfrom the external library are found ataie.extras, as before. The additional code that is useful for this project but potentially not upstream-able/applicable formlir-python-extrashas been moved toaie.helpers. I'm open to naming suggestions if there are better ideas than this!The installation of
mlir-python-extrasis not as clean as I would like, because to build the python api wheels, dependencies are installed via therequirements.txtfile; however, the requirements are parsed in a way that is incompatible with requirements installed via github repos (which is howmlir-python-extrasis installed). I think the truly correct way to fix this is to change how we handle dependencies in the wheels; I'm not sure if this is in the scope of this PR or not. I'm happy to get advice.