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

investigate __future__ import bug #4802

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

pjmagee
Copy link

@pjmagee pjmagee commented Jun 10, 2024

fixes #4600 Is this the right place to be looking?

@baywet
Copy link
Member

baywet commented Jun 12, 2024

@pjmagee yes, it's the right place. Thanks for starting this and let us know if you have questions

@pjmagee pjmagee force-pushed the __future__import-fix branch from 22b0b47 to a3f2f74 Compare June 12, 2024 22:43
@pjmagee
Copy link
Author

pjmagee commented Jun 12, 2024

This is so odd, running VS Code Python Launcher and modifying to use my spec and output, the future seems to be sorted correctly, but not when I'm using the released docker image approach.

Tempted to see if running the .NET tool variant behaves any differently. I'll look into that tomorrow if I have any spare time.

@baywet
Copy link
Member

baywet commented Jun 13, 2024

That might be because the order by, or the them by in your commented code doesn't pass a comparer. And the defaults on each platform might be slightly different

@pjmagee
Copy link
Author

pjmagee commented Jun 13, 2024

That might be because the order by, or the them by in your commented code doesn't pass a comparer. And the defaults on each platform might be slightly different

I didn't change anything, the commented code I placed in the LINQ statements to handle the _future_ was only going to be applied once i had failing testing with the existing code.

@pjmagee
Copy link
Author

pjmagee commented Jun 13, 2024

Tried using the .devcontainer to see if running inside there and running python on a linux env would do anything differently, but it was generating correctly. This makes no sense

@pjmagee
Copy link
Author

pjmagee commented Jun 13, 2024

Wow, i got around the problem, but it's strange....

So instead of using the Kiota docker image, i use a .NET SDK 8 base image, install the kiota dotnet tool and then use that to generate the python code, and the problem has gone away.

This leads me to believe it's something around culture or modifications made in the published kiota docker image which is having different behaviour than how we develop/test and run code. But that's really odd to me.

I think the default (not the code I added, i mean the original code as it is right now) OrderBy should somehow also take a specified CultureInfo of invariant? Maybe something funky is going on because this is missing, similar to what you had said about my commented out code, which I never ended up using, because i cant get the tests to fail first.

I want failing tests and then to write the solution and confirm the tests pass, struggling to do that.

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.

[Python] SyntaxError: from __future__ imports must occur at the beginning of the file
2 participants