✨ Increase number_of_types to 8 to support longer select queries - #1326

Closed
GiorgioPorgio wants to merge 2 commits into
fastapi:mainfrom
GiorgioPorgio:increase-select-overload-8-args
Closed

✨ Increase number_of_types to 8 to support longer select queries#1326
GiorgioPorgio wants to merge 2 commits into
fastapi:mainfrom
GiorgioPorgio:increase-select-overload-8-args

Conversation

@GiorgioPorgio

@GiorgioPorgioGiorgioPorgio commented Mar 19, 2025

Copy link
Copy Markdown

I am trying to write a select statement that selects 5 different SQLModel children. However I get a type error as the overloads for select are limited to 4.

I am aware there is a PR that adds typing for select for arbitrary length :)
Since that one is not merged and has conflicts (I think) I thought to give you the option to stick to the original appraoch of autogenerating types in the meantime. I increased the number of types and run the script.

Feel free to close my PR if you would rather take another approach but I think it's a quick win!

Resolves92, 271

Thank you :)

@svlandegsvlandeg added the feature New feature or request label Mar 19, 2025
@svlandegsvlandeg changed the title Increases select type overload to 8 args✨ Increase number_of_types to 8 to support longer select queriesMar 19, 2025

@svlandegsvlandeg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi, thanks for your contribution!

It looks like the CI is failing, I'm not exactly sure why though. I'll put the PR in draft until the CI is green.

Then again, I think that perhaps it makes most sense to work on the PR that has the more generic fix. The conflicts usually just mean that the PR edits the same file as another PR that has already been merged into main - it's not necessarily an actual conflict or issue.

@svlandeg
svlandeg marked this pull request as draft March 19, 2025 16:37
@GiorgioPorgio
GiorgioPorgioforce-pushed the increase-select-overload-8-args branch from 909d728 to fa686f2CompareMarch 27, 2025 14:17
@GiorgioPorgio

GiorgioPorgio commented Mar 27, 2025

Copy link
Copy Markdown
Author

Hi @svlandeg

Thank you for getting back to me!
I'm not sure if @maxispeicher is active to resolve the conflicts and I did not have permissions to work on their branch. So I just changed this branch here to apply their changes adapted to the new filenames since they opened their PR.

So this branch should be good now. Credits to @maxispeicher ofc :)

Edit: did not change my branch name because I'd have to open a new PR I think and we'd lose the conversation context

@GiorgioPorgio
GiorgioPorgio marked this pull request as ready for review March 27, 2025 14:31
@GiorgioPorgio
GiorgioPorgio marked this pull request as draft March 27, 2025 15:27
@GiorgioPorgio

Copy link
Copy Markdown
Author

After testing in a large project, I am not sure this approach can work. There are the automatically generated types such as

@overload
def select( # type: ignore
__ent0: _TCCA[_T0],
__ent1: _TCCA[_T1],
) -> Select[Tuple[_T0, _T1]]: ..

This overload would result in less specific typing and would break consumers relying on the less specific typing:

@overload
def select(*entities: _TCCA[_T0]) -> Select[Tuple[_T0, ...]]: # type: ignore
...

select(ClassA, ClassB) would be typed as Select[Tuple[ClassA | ClassB, ...]] instead of Select[Tuple[ClassA, ClassB]]

So I do not think this approach is viable.
I apologise for the confusion, I should have tested better :)

Unfortunately, I cannot think of a solution other than pre-generating types as we originally did.
I will close the PR so it won't get accidentally merged now it is approved.

Happy to pick this issue up in another PR if we find an agreeable way forward!

Thank you very much!

@svlandeg

Copy link
Copy Markdown
Member

So I do not think this approach is viable.
I apologise for the confusion, I should have tested better :)

No need to apologize, definitely appreciate the time and effort you spent, even if it's not mergeable right now! 🙏

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

featureNew feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unexpected argument(s) warning in PyCharm when using select() with many parameters

4 participants

@GiorgioPorgio@svlandeg@Seluj78@tiangolo
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

✨ Increase number_of_types to 8 to support longer select queries - #1326

Closed
GiorgioPorgio wants to merge 2 commits into
fastapi:mainfrom
GiorgioPorgio:increase-select-overload-8-args
Closed

✨ Increase number_of_types to 8 to support longer select queries#1326
GiorgioPorgio wants to merge 2 commits into
fastapi:mainfrom
GiorgioPorgio:increase-select-overload-8-args

Conversation

@GiorgioPorgio

@GiorgioPorgioGiorgioPorgio commented Mar 19, 2025

Copy link
Copy Markdown

I am trying to write a select statement that selects 5 different SQLModel children. However I get a type error as the overloads for select are limited to 4.

I am aware there is a PR that adds typing for select for arbitrary length :)
Since that one is not merged and has conflicts (I think) I thought to give you the option to stick to the original appraoch of autogenerating types in the meantime. I increased the number of types and run the script.

Feel free to close my PR if you would rather take another approach but I think it's a quick win!

Resolves92, 271

Thank you :)

@svlandegsvlandeg added the feature New feature or request label Mar 19, 2025
@svlandegsvlandeg changed the title Increases select type overload to 8 args✨ Increase number_of_types to 8 to support longer select queriesMar 19, 2025

@svlandegsvlandeg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi, thanks for your contribution!

It looks like the CI is failing, I'm not exactly sure why though. I'll put the PR in draft until the CI is green.

Then again, I think that perhaps it makes most sense to work on the PR that has the more generic fix. The conflicts usually just mean that the PR edits the same file as another PR that has already been merged into main - it's not necessarily an actual conflict or issue.

@svlandeg
svlandeg marked this pull request as draft March 19, 2025 16:37
@GiorgioPorgio
GiorgioPorgioforce-pushed the increase-select-overload-8-args branch from 909d728 to fa686f2CompareMarch 27, 2025 14:17
@GiorgioPorgio

GiorgioPorgio commented Mar 27, 2025

Copy link
Copy Markdown
Author

Hi @svlandeg

Thank you for getting back to me!
I'm not sure if @maxispeicher is active to resolve the conflicts and I did not have permissions to work on their branch. So I just changed this branch here to apply their changes adapted to the new filenames since they opened their PR.

So this branch should be good now. Credits to @maxispeicher ofc :)

Edit: did not change my branch name because I'd have to open a new PR I think and we'd lose the conversation context

@GiorgioPorgio
GiorgioPorgio marked this pull request as ready for review March 27, 2025 14:31
@GiorgioPorgio
GiorgioPorgio marked this pull request as draft March 27, 2025 15:27
@GiorgioPorgio

Copy link
Copy Markdown
Author

After testing in a large project, I am not sure this approach can work. There are the automatically generated types such as

@overload
def select( # type: ignore
__ent0: _TCCA[_T0],
__ent1: _TCCA[_T1],
) -> Select[Tuple[_T0, _T1]]: ..

This overload would result in less specific typing and would break consumers relying on the less specific typing:

@overload
def select(*entities: _TCCA[_T0]) -> Select[Tuple[_T0, ...]]: # type: ignore
...

select(ClassA, ClassB) would be typed as Select[Tuple[ClassA | ClassB, ...]] instead of Select[Tuple[ClassA, ClassB]]

So I do not think this approach is viable.
I apologise for the confusion, I should have tested better :)

Unfortunately, I cannot think of a solution other than pre-generating types as we originally did.
I will close the PR so it won't get accidentally merged now it is approved.

Happy to pick this issue up in another PR if we find an agreeable way forward!

Thank you very much!

@svlandeg

Copy link
Copy Markdown
Member

So I do not think this approach is viable.
I apologise for the confusion, I should have tested better :)

No need to apologize, definitely appreciate the time and effort you spent, even if it's not mergeable right now! 🙏

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

featureNew feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unexpected argument(s) warning in PyCharm when using select() with many parameters

4 participants

@GiorgioPorgio@svlandeg@Seluj78@tiangolo
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

✨ Increase number_of_types to 8 to support longer select queries - #1326

Closed
GiorgioPorgio wants to merge 2 commits into
fastapi:mainfrom
GiorgioPorgio:increase-select-overload-8-args
Closed

✨ Increase number_of_types to 8 to support longer select queries#1326
GiorgioPorgio wants to merge 2 commits into
fastapi:mainfrom
GiorgioPorgio:increase-select-overload-8-args

Conversation

@GiorgioPorgio

@GiorgioPorgioGiorgioPorgio commented Mar 19, 2025

Copy link
Copy Markdown

I am trying to write a select statement that selects 5 different SQLModel children. However I get a type error as the overloads for select are limited to 4.

I am aware there is a PR that adds typing for select for arbitrary length :)
Since that one is not merged and has conflicts (I think) I thought to give you the option to stick to the original appraoch of autogenerating types in the meantime. I increased the number of types and run the script.

Feel free to close my PR if you would rather take another approach but I think it's a quick win!

Resolves92, 271

Thank you :)

@svlandegsvlandeg added the feature New feature or request label Mar 19, 2025
@svlandegsvlandeg changed the title Increases select type overload to 8 args✨ Increase number_of_types to 8 to support longer select queriesMar 19, 2025

@svlandegsvlandeg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi, thanks for your contribution!

It looks like the CI is failing, I'm not exactly sure why though. I'll put the PR in draft until the CI is green.

Then again, I think that perhaps it makes most sense to work on the PR that has the more generic fix. The conflicts usually just mean that the PR edits the same file as another PR that has already been merged into main - it's not necessarily an actual conflict or issue.

@svlandeg
svlandeg marked this pull request as draft March 19, 2025 16:37
@GiorgioPorgio
GiorgioPorgioforce-pushed the increase-select-overload-8-args branch from 909d728 to fa686f2CompareMarch 27, 2025 14:17
@GiorgioPorgio

GiorgioPorgio commented Mar 27, 2025

Copy link
Copy Markdown
Author

Hi @svlandeg

Thank you for getting back to me!
I'm not sure if @maxispeicher is active to resolve the conflicts and I did not have permissions to work on their branch. So I just changed this branch here to apply their changes adapted to the new filenames since they opened their PR.

So this branch should be good now. Credits to @maxispeicher ofc :)

Edit: did not change my branch name because I'd have to open a new PR I think and we'd lose the conversation context

@GiorgioPorgio
GiorgioPorgio marked this pull request as ready for review March 27, 2025 14:31
@GiorgioPorgio
GiorgioPorgio marked this pull request as draft March 27, 2025 15:27
@GiorgioPorgio

Copy link
Copy Markdown
Author

After testing in a large project, I am not sure this approach can work. There are the automatically generated types such as

@overload
def select( # type: ignore
__ent0: _TCCA[_T0],
__ent1: _TCCA[_T1],
) -> Select[Tuple[_T0, _T1]]: ..

This overload would result in less specific typing and would break consumers relying on the less specific typing:

@overload
def select(*entities: _TCCA[_T0]) -> Select[Tuple[_T0, ...]]: # type: ignore
...

select(ClassA, ClassB) would be typed as Select[Tuple[ClassA | ClassB, ...]] instead of Select[Tuple[ClassA, ClassB]]

So I do not think this approach is viable.
I apologise for the confusion, I should have tested better :)

Unfortunately, I cannot think of a solution other than pre-generating types as we originally did.
I will close the PR so it won't get accidentally merged now it is approved.

Happy to pick this issue up in another PR if we find an agreeable way forward!

Thank you very much!

@svlandeg

Copy link
Copy Markdown
Member

So I do not think this approach is viable.
I apologise for the confusion, I should have tested better :)

No need to apologize, definitely appreciate the time and effort you spent, even if it's not mergeable right now! 🙏

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

featureNew feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unexpected argument(s) warning in PyCharm when using select() with many parameters

4 participants

@GiorgioPorgio@svlandeg@Seluj78@tiangolo
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

✨ Increase number_of_types to 8 to support longer select queries - #1326

Closed
GiorgioPorgio wants to merge 2 commits into
fastapi:mainfrom
GiorgioPorgio:increase-select-overload-8-args
Closed

✨ Increase number_of_types to 8 to support longer select queries#1326
GiorgioPorgio wants to merge 2 commits into
fastapi:mainfrom
GiorgioPorgio:increase-select-overload-8-args

Conversation

@GiorgioPorgio

@GiorgioPorgioGiorgioPorgio commented Mar 19, 2025

Copy link
Copy Markdown

I am trying to write a select statement that selects 5 different SQLModel children. However I get a type error as the overloads for select are limited to 4.

I am aware there is a PR that adds typing for select for arbitrary length :)
Since that one is not merged and has conflicts (I think) I thought to give you the option to stick to the original appraoch of autogenerating types in the meantime. I increased the number of types and run the script.

Feel free to close my PR if you would rather take another approach but I think it's a quick win!

Resolves92, 271

Thank you :)

@svlandegsvlandeg added the feature New feature or request label Mar 19, 2025
@svlandegsvlandeg changed the title Increases select type overload to 8 args✨ Increase number_of_types to 8 to support longer select queriesMar 19, 2025

@svlandegsvlandeg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi, thanks for your contribution!

It looks like the CI is failing, I'm not exactly sure why though. I'll put the PR in draft until the CI is green.

Then again, I think that perhaps it makes most sense to work on the PR that has the more generic fix. The conflicts usually just mean that the PR edits the same file as another PR that has already been merged into main - it's not necessarily an actual conflict or issue.

@svlandeg
svlandeg marked this pull request as draft March 19, 2025 16:37
@GiorgioPorgio
GiorgioPorgioforce-pushed the increase-select-overload-8-args branch from 909d728 to fa686f2CompareMarch 27, 2025 14:17
@GiorgioPorgio

GiorgioPorgio commented Mar 27, 2025

Copy link
Copy Markdown
Author

Hi @svlandeg

Thank you for getting back to me!
I'm not sure if @maxispeicher is active to resolve the conflicts and I did not have permissions to work on their branch. So I just changed this branch here to apply their changes adapted to the new filenames since they opened their PR.

So this branch should be good now. Credits to @maxispeicher ofc :)

Edit: did not change my branch name because I'd have to open a new PR I think and we'd lose the conversation context

@GiorgioPorgio
GiorgioPorgio marked this pull request as ready for review March 27, 2025 14:31
@GiorgioPorgio
GiorgioPorgio marked this pull request as draft March 27, 2025 15:27
@GiorgioPorgio

Copy link
Copy Markdown
Author

After testing in a large project, I am not sure this approach can work. There are the automatically generated types such as

@overload
def select( # type: ignore
__ent0: _TCCA[_T0],
__ent1: _TCCA[_T1],
) -> Select[Tuple[_T0, _T1]]: ..

This overload would result in less specific typing and would break consumers relying on the less specific typing:

@overload
def select(*entities: _TCCA[_T0]) -> Select[Tuple[_T0, ...]]: # type: ignore
...

select(ClassA, ClassB) would be typed as Select[Tuple[ClassA | ClassB, ...]] instead of Select[Tuple[ClassA, ClassB]]

So I do not think this approach is viable.
I apologise for the confusion, I should have tested better :)

Unfortunately, I cannot think of a solution other than pre-generating types as we originally did.
I will close the PR so it won't get accidentally merged now it is approved.

Happy to pick this issue up in another PR if we find an agreeable way forward!

Thank you very much!

@svlandeg

Copy link
Copy Markdown
Member

So I do not think this approach is viable.
I apologise for the confusion, I should have tested better :)

No need to apologize, definitely appreciate the time and effort you spent, even if it's not mergeable right now! 🙏

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

featureNew feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unexpected argument(s) warning in PyCharm when using select() with many parameters

4 participants

@GiorgioPorgio@svlandeg@Seluj78@tiangolo
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

✨ Increase number_of_types to 8 to support longer select queries - #1326

Closed
GiorgioPorgio wants to merge 2 commits into
fastapi:mainfrom
GiorgioPorgio:increase-select-overload-8-args
Closed

✨ Increase number_of_types to 8 to support longer select queries#1326
GiorgioPorgio wants to merge 2 commits into
fastapi:mainfrom
GiorgioPorgio:increase-select-overload-8-args

Conversation

@GiorgioPorgio

@GiorgioPorgioGiorgioPorgio commented Mar 19, 2025

Copy link
Copy Markdown

I am trying to write a select statement that selects 5 different SQLModel children. However I get a type error as the overloads for select are limited to 4.

I am aware there is a PR that adds typing for select for arbitrary length :)
Since that one is not merged and has conflicts (I think) I thought to give you the option to stick to the original appraoch of autogenerating types in the meantime. I increased the number of types and run the script.

Feel free to close my PR if you would rather take another approach but I think it's a quick win!

Resolves92, 271

Thank you :)

@svlandegsvlandeg added the feature New feature or request label Mar 19, 2025
@svlandegsvlandeg changed the title Increases select type overload to 8 args✨ Increase number_of_types to 8 to support longer select queriesMar 19, 2025

@svlandegsvlandeg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi, thanks for your contribution!

It looks like the CI is failing, I'm not exactly sure why though. I'll put the PR in draft until the CI is green.

Then again, I think that perhaps it makes most sense to work on the PR that has the more generic fix. The conflicts usually just mean that the PR edits the same file as another PR that has already been merged into main - it's not necessarily an actual conflict or issue.

@svlandeg
svlandeg marked this pull request as draft March 19, 2025 16:37
@GiorgioPorgio
GiorgioPorgioforce-pushed the increase-select-overload-8-args branch from 909d728 to fa686f2CompareMarch 27, 2025 14:17
@GiorgioPorgio

GiorgioPorgio commented Mar 27, 2025

Copy link
Copy Markdown
Author

Hi @svlandeg

Thank you for getting back to me!
I'm not sure if @maxispeicher is active to resolve the conflicts and I did not have permissions to work on their branch. So I just changed this branch here to apply their changes adapted to the new filenames since they opened their PR.

So this branch should be good now. Credits to @maxispeicher ofc :)

Edit: did not change my branch name because I'd have to open a new PR I think and we'd lose the conversation context

@GiorgioPorgio
GiorgioPorgio marked this pull request as ready for review March 27, 2025 14:31
@GiorgioPorgio
GiorgioPorgio marked this pull request as draft March 27, 2025 15:27
@GiorgioPorgio

Copy link
Copy Markdown
Author

After testing in a large project, I am not sure this approach can work. There are the automatically generated types such as

@overload
def select( # type: ignore
__ent0: _TCCA[_T0],
__ent1: _TCCA[_T1],
) -> Select[Tuple[_T0, _T1]]: ..

This overload would result in less specific typing and would break consumers relying on the less specific typing:

@overload
def select(*entities: _TCCA[_T0]) -> Select[Tuple[_T0, ...]]: # type: ignore
...

select(ClassA, ClassB) would be typed as Select[Tuple[ClassA | ClassB, ...]] instead of Select[Tuple[ClassA, ClassB]]

So I do not think this approach is viable.
I apologise for the confusion, I should have tested better :)

Unfortunately, I cannot think of a solution other than pre-generating types as we originally did.
I will close the PR so it won't get accidentally merged now it is approved.

Happy to pick this issue up in another PR if we find an agreeable way forward!

Thank you very much!

@svlandeg

Copy link
Copy Markdown
Member

So I do not think this approach is viable.
I apologise for the confusion, I should have tested better :)

No need to apologize, definitely appreciate the time and effort you spent, even if it's not mergeable right now! 🙏

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

featureNew feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unexpected argument(s) warning in PyCharm when using select() with many parameters

4 participants

@GiorgioPorgio@svlandeg@Seluj78@tiangolo
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

✨ Increase number_of_types to 8 to support longer select queries - #1326

Closed
GiorgioPorgio wants to merge 2 commits into
fastapi:mainfrom
GiorgioPorgio:increase-select-overload-8-args
Closed

✨ Increase number_of_types to 8 to support longer select queries#1326
GiorgioPorgio wants to merge 2 commits into
fastapi:mainfrom
GiorgioPorgio:increase-select-overload-8-args

Conversation

@GiorgioPorgio

@GiorgioPorgioGiorgioPorgio commented Mar 19, 2025

Copy link
Copy Markdown

I am trying to write a select statement that selects 5 different SQLModel children. However I get a type error as the overloads for select are limited to 4.

I am aware there is a PR that adds typing for select for arbitrary length :)
Since that one is not merged and has conflicts (I think) I thought to give you the option to stick to the original appraoch of autogenerating types in the meantime. I increased the number of types and run the script.

Feel free to close my PR if you would rather take another approach but I think it's a quick win!

Resolves92, 271

Thank you :)

@svlandegsvlandeg added the feature New feature or request label Mar 19, 2025
@svlandegsvlandeg changed the title Increases select type overload to 8 args✨ Increase number_of_types to 8 to support longer select queriesMar 19, 2025

@svlandegsvlandeg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi, thanks for your contribution!

It looks like the CI is failing, I'm not exactly sure why though. I'll put the PR in draft until the CI is green.

Then again, I think that perhaps it makes most sense to work on the PR that has the more generic fix. The conflicts usually just mean that the PR edits the same file as another PR that has already been merged into main - it's not necessarily an actual conflict or issue.

@svlandeg
svlandeg marked this pull request as draft March 19, 2025 16:37
@GiorgioPorgio
GiorgioPorgioforce-pushed the increase-select-overload-8-args branch from 909d728 to fa686f2CompareMarch 27, 2025 14:17
@GiorgioPorgio

GiorgioPorgio commented Mar 27, 2025

Copy link
Copy Markdown
Author

Hi @svlandeg

Thank you for getting back to me!
I'm not sure if @maxispeicher is active to resolve the conflicts and I did not have permissions to work on their branch. So I just changed this branch here to apply their changes adapted to the new filenames since they opened their PR.

So this branch should be good now. Credits to @maxispeicher ofc :)

Edit: did not change my branch name because I'd have to open a new PR I think and we'd lose the conversation context

@GiorgioPorgio
GiorgioPorgio marked this pull request as ready for review March 27, 2025 14:31
@GiorgioPorgio
GiorgioPorgio marked this pull request as draft March 27, 2025 15:27
@GiorgioPorgio

Copy link
Copy Markdown
Author

After testing in a large project, I am not sure this approach can work. There are the automatically generated types such as

@overload
def select( # type: ignore
__ent0: _TCCA[_T0],
__ent1: _TCCA[_T1],
) -> Select[Tuple[_T0, _T1]]: ..

This overload would result in less specific typing and would break consumers relying on the less specific typing:

@overload
def select(*entities: _TCCA[_T0]) -> Select[Tuple[_T0, ...]]: # type: ignore
...

select(ClassA, ClassB) would be typed as Select[Tuple[ClassA | ClassB, ...]] instead of Select[Tuple[ClassA, ClassB]]

So I do not think this approach is viable.
I apologise for the confusion, I should have tested better :)

Unfortunately, I cannot think of a solution other than pre-generating types as we originally did.
I will close the PR so it won't get accidentally merged now it is approved.

Happy to pick this issue up in another PR if we find an agreeable way forward!

Thank you very much!

@svlandeg

Copy link
Copy Markdown
Member

So I do not think this approach is viable.
I apologise for the confusion, I should have tested better :)

No need to apologize, definitely appreciate the time and effort you spent, even if it's not mergeable right now! 🙏

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

featureNew feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unexpected argument(s) warning in PyCharm when using select() with many parameters

4 participants

@GiorgioPorgio@svlandeg@Seluj78@tiangolo
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

✨ Increase number_of_types to 8 to support longer select queries - #1326

Closed
GiorgioPorgio wants to merge 2 commits into
fastapi:mainfrom
GiorgioPorgio:increase-select-overload-8-args
Closed

✨ Increase number_of_types to 8 to support longer select queries#1326
GiorgioPorgio wants to merge 2 commits into
fastapi:mainfrom
GiorgioPorgio:increase-select-overload-8-args

Conversation

@GiorgioPorgio

@GiorgioPorgioGiorgioPorgio commented Mar 19, 2025

Copy link
Copy Markdown

I am trying to write a select statement that selects 5 different SQLModel children. However I get a type error as the overloads for select are limited to 4.

I am aware there is a PR that adds typing for select for arbitrary length :)
Since that one is not merged and has conflicts (I think) I thought to give you the option to stick to the original appraoch of autogenerating types in the meantime. I increased the number of types and run the script.

Feel free to close my PR if you would rather take another approach but I think it's a quick win!

Resolves92, 271

Thank you :)

@svlandegsvlandeg added the feature New feature or request label Mar 19, 2025
@svlandegsvlandeg changed the title Increases select type overload to 8 args✨ Increase number_of_types to 8 to support longer select queriesMar 19, 2025

@svlandegsvlandeg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi, thanks for your contribution!

It looks like the CI is failing, I'm not exactly sure why though. I'll put the PR in draft until the CI is green.

Then again, I think that perhaps it makes most sense to work on the PR that has the more generic fix. The conflicts usually just mean that the PR edits the same file as another PR that has already been merged into main - it's not necessarily an actual conflict or issue.

@svlandeg
svlandeg marked this pull request as draft March 19, 2025 16:37
@GiorgioPorgio
GiorgioPorgioforce-pushed the increase-select-overload-8-args branch from 909d728 to fa686f2CompareMarch 27, 2025 14:17
@GiorgioPorgio

GiorgioPorgio commented Mar 27, 2025

Copy link
Copy Markdown
Author

Hi @svlandeg

Thank you for getting back to me!
I'm not sure if @maxispeicher is active to resolve the conflicts and I did not have permissions to work on their branch. So I just changed this branch here to apply their changes adapted to the new filenames since they opened their PR.

So this branch should be good now. Credits to @maxispeicher ofc :)

Edit: did not change my branch name because I'd have to open a new PR I think and we'd lose the conversation context

@GiorgioPorgio
GiorgioPorgio marked this pull request as ready for review March 27, 2025 14:31
@GiorgioPorgio
GiorgioPorgio marked this pull request as draft March 27, 2025 15:27
@GiorgioPorgio

Copy link
Copy Markdown
Author

After testing in a large project, I am not sure this approach can work. There are the automatically generated types such as

@overload
def select( # type: ignore
__ent0: _TCCA[_T0],
__ent1: _TCCA[_T1],
) -> Select[Tuple[_T0, _T1]]: ..

This overload would result in less specific typing and would break consumers relying on the less specific typing:

@overload
def select(*entities: _TCCA[_T0]) -> Select[Tuple[_T0, ...]]: # type: ignore
...

select(ClassA, ClassB) would be typed as Select[Tuple[ClassA | ClassB, ...]] instead of Select[Tuple[ClassA, ClassB]]

So I do not think this approach is viable.
I apologise for the confusion, I should have tested better :)

Unfortunately, I cannot think of a solution other than pre-generating types as we originally did.
I will close the PR so it won't get accidentally merged now it is approved.

Happy to pick this issue up in another PR if we find an agreeable way forward!

Thank you very much!

@svlandeg

Copy link
Copy Markdown
Member

So I do not think this approach is viable.
I apologise for the confusion, I should have tested better :)

No need to apologize, definitely appreciate the time and effort you spent, even if it's not mergeable right now! 🙏

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

featureNew feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unexpected argument(s) warning in PyCharm when using select() with many parameters

4 participants

@GiorgioPorgio@svlandeg@Seluj78@tiangolo
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

✨ Increase number_of_types to 8 to support longer select queries - #1326

Closed
GiorgioPorgio wants to merge 2 commits into
fastapi:mainfrom
GiorgioPorgio:increase-select-overload-8-args
Closed

✨ Increase number_of_types to 8 to support longer select queries#1326
GiorgioPorgio wants to merge 2 commits into
fastapi:mainfrom
GiorgioPorgio:increase-select-overload-8-args

Conversation

@GiorgioPorgio

@GiorgioPorgioGiorgioPorgio commented Mar 19, 2025

Copy link
Copy Markdown

I am trying to write a select statement that selects 5 different SQLModel children. However I get a type error as the overloads for select are limited to 4.

I am aware there is a PR that adds typing for select for arbitrary length :)
Since that one is not merged and has conflicts (I think) I thought to give you the option to stick to the original appraoch of autogenerating types in the meantime. I increased the number of types and run the script.

Feel free to close my PR if you would rather take another approach but I think it's a quick win!

Resolves92, 271

Thank you :)

@svlandegsvlandeg added the feature New feature or request label Mar 19, 2025
@svlandegsvlandeg changed the title Increases select type overload to 8 args✨ Increase number_of_types to 8 to support longer select queriesMar 19, 2025

@svlandegsvlandeg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi, thanks for your contribution!

It looks like the CI is failing, I'm not exactly sure why though. I'll put the PR in draft until the CI is green.

Then again, I think that perhaps it makes most sense to work on the PR that has the more generic fix. The conflicts usually just mean that the PR edits the same file as another PR that has already been merged into main - it's not necessarily an actual conflict or issue.

@svlandeg
svlandeg marked this pull request as draft March 19, 2025 16:37
@GiorgioPorgio
GiorgioPorgioforce-pushed the increase-select-overload-8-args branch from 909d728 to fa686f2CompareMarch 27, 2025 14:17
@GiorgioPorgio

GiorgioPorgio commented Mar 27, 2025

Copy link
Copy Markdown
Author

Hi @svlandeg

Thank you for getting back to me!
I'm not sure if @maxispeicher is active to resolve the conflicts and I did not have permissions to work on their branch. So I just changed this branch here to apply their changes adapted to the new filenames since they opened their PR.

So this branch should be good now. Credits to @maxispeicher ofc :)

Edit: did not change my branch name because I'd have to open a new PR I think and we'd lose the conversation context

@GiorgioPorgio
GiorgioPorgio marked this pull request as ready for review March 27, 2025 14:31
@GiorgioPorgio
GiorgioPorgio marked this pull request as draft March 27, 2025 15:27
@GiorgioPorgio

Copy link
Copy Markdown
Author

After testing in a large project, I am not sure this approach can work. There are the automatically generated types such as

@overload
def select( # type: ignore
__ent0: _TCCA[_T0],
__ent1: _TCCA[_T1],
) -> Select[Tuple[_T0, _T1]]: ..

This overload would result in less specific typing and would break consumers relying on the less specific typing:

@overload
def select(*entities: _TCCA[_T0]) -> Select[Tuple[_T0, ...]]: # type: ignore
...

select(ClassA, ClassB) would be typed as Select[Tuple[ClassA | ClassB, ...]] instead of Select[Tuple[ClassA, ClassB]]

So I do not think this approach is viable.
I apologise for the confusion, I should have tested better :)

Unfortunately, I cannot think of a solution other than pre-generating types as we originally did.
I will close the PR so it won't get accidentally merged now it is approved.

Happy to pick this issue up in another PR if we find an agreeable way forward!

Thank you very much!

@svlandeg

Copy link
Copy Markdown
Member

So I do not think this approach is viable.
I apologise for the confusion, I should have tested better :)

No need to apologize, definitely appreciate the time and effort you spent, even if it's not mergeable right now! 🙏

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

featureNew feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unexpected argument(s) warning in PyCharm when using select() with many parameters

4 participants

@GiorgioPorgio@svlandeg@Seluj78@tiangolo