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

Import statements fixed in example #88

Merged
merged 3 commits into from
Mar 12, 2022

Conversation

Piyush13y
Copy link
Collaborator

@Piyush13y Piyush13y commented Feb 20, 2022

This tiny PR fixes #82

Description of changes

import statements fixed

Possible influences of this PR.

The example shouldn't throw ModuleNotFound exception anymore.

@Piyush13y Piyush13y changed the title Huggingface import statements fixed Import statements fixed Feb 20, 2022
@Piyush13y Piyush13y changed the title Import statements fixed Import statements fixed in example Feb 20, 2022
Copy link
Member

@hunterhector hunterhector left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix.

As for the mypy error, this is due to we updated numpy version in texar, which now contains its own typing files. So we would need to fix a few typing issues accordingly.

It might not be possible to fix typing for all different python version and numpy version, it is sufficient to only test on the newest numpy and newest python version.

So we would need to change this:
https://github.com/asyml/forte-wrappers/blob/main/.github/workflows/main.yml#L68

to something similar as:

https://github.com/asyml/forte/blob/master/.github/workflows/main.yml#L94

@Piyush13y
Copy link
Collaborator Author

@hunterhector Is a fix for this typing issue in progress? Or do you want me to pick it? Because there are more changes coming in to Forte-wrappers related to the ontology namespace consistency.

@hunterhector
Copy link
Member

@hunterhector Is a fix for this typing issue in progress? Or do you want me to pick it? Because there are more changes coming in to Forte-wrappers related to the ontology namespace consistency.

We don't have anything in progress for forte-wrapper now, you probably need to study this.

@hunterhector
Copy link
Member

hunterhector commented Mar 1, 2022

Updates on this. We have updated the texar-pytorch side with numpy typings. But now the new numpy versions causes a few typing errors in this repo that need to be fixed too.

Pengfei is working on them (#90) but may not finish it today. So if you are in a hurry you can try to deal with it yourself. Otherwise this might get fixed around tomorrow.

@Piyush13y
Copy link
Collaborator Author

Thanks for the information. I don't feel this is a priority change. We can wait till the fix is out, I will re-request a review, once it passes all the checks.

@Piyush13y Piyush13y requested a review from hunterhector March 12, 2022 00:14
@hunterhector hunterhector merged commit 0763d3a into asyml:main Mar 12, 2022
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.

Huggingface module imported incorrectly
2 participants