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

[Modelina] Add support for Python 3.x #975

Closed
avineshwar opened this issue Oct 26, 2022 · 8 comments
Closed

[Modelina] Add support for Python 3.x #975

avineshwar opened this issue Oct 26, 2022 · 8 comments
Labels
enhancement New feature or request

Comments

@avineshwar
Copy link

Reason/Context

Please try answering few of those questions

  • Why we need this improvement?
  • How will this change help?
  • What is the motivation?

Description

Please try answering few of those questions

  • What changes have to be introduced?
  • Will this be a breaking change?
  • How could it be implemented/designed?
@avineshwar avineshwar added the enhancement New feature or request label Oct 26, 2022
@github-actions
Copy link
Contributor

Welcome to AsyncAPI. Thanks a lot for reporting your first issue. Please check out our contributors guide and the instructions about a basic recommended setup useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

@jonaslagoni
Copy link
Member

@avineshwar have you checked out the next version?

If you have, do you mind clarifying how the current python output is insufficient to your liking?

@zaytsevand
Copy link
Contributor

I guess it's about Implicit-related imports which will not work with Python 3.3+:
PEP-328
Changed in version 3.3

@jonaslagoni
Copy link
Member

That should be manageable, we also have different import ways for TypeScript.

@zaytsevand
Copy link
Contributor

zaytsevand commented Oct 31, 2022

That should be manageable, we also have different import ways for TypeScript.

Could you please suggest a proper way to implement an import differently?
My dumb workaround for the issue is:

const models = await generator.generateCompleteModels(spec, {});
const libraryDependencies = new Set(models.flatMap(m => m.dependencies));

for (const model of models) {
    await writeFile(`${outputDirectory}/${model.modelName}.py`,
        model.result
            .split('\n')
            .map(s => {
                if (libraryDependencies.has(s)) {
                    return s;
                }
                if (s.startsWith('from ')) {
                    return s.replace('from ', 'from .');
                }
                return s;
            })
            .map(s => `${s}\n`)
            .filter(s => s !== '\n'));
}
  1. If it's a library dep, skip it altogether. Unfortunately, here I'm working with dependencies on an import statement level, not with package names (which would be a much cleaner alternative)
  2. If it's any other import, then prefix it with a dot, assuming that here we won't receive anything but local imports

@jonaslagoni
Copy link
Member

If the generateCompleteModels function does not generate accurate imports, then we should change how it renders dependencies.

Do you have a suggestion for an option you can provide with the function? That we can use inside the python renderer to render appropriate dependencies?

@zaytsevand
Copy link
Contributor

zaytsevand commented Oct 31, 2022

@jonaslagoni please check the attached PR. Hope that it would do the trick #981

@jonaslagoni
Copy link
Member

I am gonna close this issue for now as a fix is in place.

Feel free to reopen @avineshwar if you don't find it solved it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants