Skip to content
This repository was archived by the owner on Aug 19, 2025. It is now read-only.

fix Row imports#376

Merged
aminalaee merged 1 commit intomasterfrom
fix-imports-and-query-type
Aug 28, 2021
Merged

fix Row imports#376
aminalaee merged 1 commit intomasterfrom
fix-imports-and-query-type

Conversation

@aminalaee
Copy link
Copy Markdown
Contributor

@aminalaee aminalaee commented Aug 27, 2021

  • Fix Row imports from SQLAlchemy
  • Rename output of query, _, _ = compile(query) to query_str, _, _ = compile(query)

@aminalaee aminalaee requested a review from a team August 27, 2021 10:31
@PrettyWood
Copy link
Copy Markdown
Contributor

@aminalaee Why this change? They changed in sqlalchemy? My PR was wrong ? If that's the case surely we could have an extra test to ensure the behaviour was wrong and is now good.
Sorry I'm just trying to understand the context

@aminalaee
Copy link
Copy Markdown
Contributor Author

aminalaee commented Aug 27, 2021

@PrettyWood
Right now Row is imported from from sqlalchemy.engine.result
But Row is not defined in there, it's just imported in result.py and it's defined in sqlalchemy.engine.row.
If you jut click on the current Row import you will go to sqlalchemy.engine.row not engine.result.

It's a matter of formatting, and the code is working alright.

P.S.: I don't know if SQLAlchemy changed that or not, but that works ok because Row is relatively imported in sqlalchemy.engine.result so it can work but I think IDEs will complain about that import.

Sorry but it wasn't anything personal, I was working on another IDE error and found this too.

@PrettyWood
Copy link
Copy Markdown
Contributor

Sorry but it wasn't anything personal,

Haha don't worry 😉 I wasn't concerned about anything like this. Was just surprised tests were working fine if the import was wrong. Thanks for clarifying!

@aminalaee aminalaee merged commit b4cd10c into master Aug 28, 2021
@aminalaee aminalaee deleted the fix-imports-and-query-type branch August 28, 2021 06:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants