Skip to content

proposal: Standardize isnot & notin_ operator names for SqlAlchemy2.0 (1.4?) #5429

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.

Sign up for GitHub

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

Jump to bottom
Closed
jvanasco opened this issue Jun 29, 2020 · 10 comments
Closed

proposal: Standardize isnot & notin_ operator names for SqlAlchemy2.0 (1.4?) #5429

jvanasco opened this issue Jun 29, 2020 · 10 comments
Assignees
@jvanasco
Labels
sql task
Milestone
1.4

Comments

@jvanasco
Copy link
Member

jvanasco commented Jun 29, 2020 •
edited
Loading

A handful of sql operators, as implemented in Python, are not consistent in SQLAlchemy.

The majority of items use a schema of snake_case_with_trailing_underscore_ when the sql command mimics Python reserved keywords

For example, some of the commands adopting this scheme use:

Python Sql SQLAlchemy
and AND and_
or OR or_
is IS is_
in IS in_

but compound operators either do not follow the same scheme, do not have an internal underscore, or both

Python Sql SQLAlchemy
is not IS NOT isnot
not in NOT IN notin_

This ticket will list them, and serve as reference to a PR if we proceed with any re-namings.

SQLAchemy 1.3 Proposed A Proposed B (rejected)
isnot is_not is_not_
notin_ not_in not_in_

If a change were to happen:

  • the core function will be renamed
  • the old function will be an alias for backwards compatibility; it will not be deprecated or warnings
  • docs will be updated to use the new terms
  • API docs for the old terms will state they are kept for backwards compatibility, and promote usage of the new terms
@jvanasco jvanasco added the requires triage New issue that requires categorization label Jun 29, 2020
@zzzeek zzzeek added task and removed requires triage New issue that requires categorization labels Jun 29, 2020
@zzzeek zzzeek added this to the 1.4 milestone Jun 29, 2020
@zzzeek zzzeek added the sql label Jun 29, 2020
@CaselIT
Copy link
Member

CaselIT commented Jun 30, 2020 •
edited
Loading

I'm not particularly fond of is_not_ and not_in_ vs is_not and not_in, but I understand the consistency of having the tailing undersoce

@jvanasco jvanasco changed the title Feature: Standardize operator names for SqlAlchemy2.0 (1.4?) Feature: Standardize isnot & notin_ operator names for SqlAlchemy2.0 (1.4?) Jul 2, 2020
@jvanasco jvanasco mentioned this issue Jul 2, 2020
Closed
@zzzeek
Copy link
Member

zzzeek commented Jul 2, 2020

just FTR I was looking to go from Python's example:

>>> import operator
>>> operator.is_
<built-in function is_>
>>> operator.is_not
<built-in function is_not>

@jvanasco
Copy link
Member Author

jvanasco commented Jul 2, 2020

@zzzeek @CaselIT I marked my tailing underscore proposal as rejected.

@zzzeek
Copy link
Member

zzzeek commented Jul 2, 2020

we can still ask twitter I often get a lot of insight from that. either way

@jvanasco jvanasco changed the title Feature: Standardize isnot & notin_ operator names for SqlAlchemy2.0 (1.4?) proposal: Standardize isnot & notin_ operator names for SqlAlchemy2.0 (1.4?) Jul 2, 2020
@xarg
Copy link

xarg commented Jul 2, 2020

The convention could be: trailing underscore should be reserved only for python single-word keywords.

Ex:

  • is -> is_
  • is not -> is_not

Also compatible with the operator module -> less surprises.

@CaselIT CaselIT pinned this issue Jul 2, 2020
@CaselIT
Copy link
Member

CaselIT commented Jul 2, 2020

I've pinned this issue and 5435, so maybe we gather some other comment from the community

@CaselIT CaselIT unpinned this issue Jul 18, 2020
@jvanasco jvanasco self-assigned this Aug 6, 2020
@jvanasco
Copy link
Member Author

jvanasco commented Aug 6, 2020

PR forthcoming...

@jvanasco jvanasco mentioned this issue Aug 7, 2020
Closed
3 tasks
jvanasco added a commit to jvanasco/sqlalchemy that referenced this issue Aug 20, 2020
@jvanasco
Issue sqlalchemy#5429 version 2: Patch Set A: internal test file

Verified

This commit was signed with the committer’s verified signature.
lukekarrys Luke Karrys
SSH Key Fingerprint: tdR2129cIUhOKMlhHfQ2KdWrwNRNDT0jNNCQR+Mbb3g
Verified
Learn about vigilant mode
ede1902
jvanasco added a commit to jvanasco/sqlalchemy that referenced this issue Aug 20, 2020
@jvanasco
Issue sqlalchemy#5429 version 2: Patch Set B: core files

Verified

This commit was signed with the committer’s verified signature.
lukekarrys Luke Karrys
SSH Key Fingerprint: tdR2129cIUhOKMlhHfQ2KdWrwNRNDT0jNNCQR+Mbb3g
Verified
Learn about vigilant mode
f2b1186
This was referenced Aug 20, 2020
Closed
Closed
@sqla-tester
Copy link
Collaborator

jonathan vanasco has proposed a fix for this issue in the master branch:

Issue #5429 version 2: Patch Set A: internal test files https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/2169

@sqla-tester
Copy link
Collaborator

jonathan vanasco has proposed a fix for this issue in the master branch:

Issue #5429 version 2: Patch Set B: core library https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/2170

jvanasco added a commit to jvanasco/sqlalchemy that referenced this issue Aug 21, 2020
@jvanasco
Issue sqlalchemy#5429 version 2: Patch Set B: core files
4d1d876 
Change-Id: I47ab0857a8f9e2db4d459c3d6b5b74985eec29ab
jvanasco added a commit to jvanasco/sqlalchemy that referenced this issue Aug 24, 2020
@jvanasco
Issue sqlalchemy#5429 version 2: Patch Set A: internal test file
e205060 
Change-Id: I8350607544f3005b1afe3bfe4f27efd740e23e82
jvanasco added a commit to jvanasco/sqlalchemy that referenced this issue Aug 24, 2020
@jvanasco
Issue sqlalchemy#5429 version 2: Patch Set A: internal test file
893e265 
Change-Id: I8350607544f3005b1afe3bfe4f27efd740e23e82
@sqla-tester
Copy link
Collaborator

jonathan vanasco has proposed a fix for this issue in the master branch:

Issue #5429 version 2: Patch Set B: core library https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/2200

jvanasco added a commit to jvanasco/sqlalchemy that referenced this issue Aug 31, 2020
@jvanasco
Issue sqlalchemy#5429 version 2: Patch Set B: core library
79c5ce3 
Revised PR of sqlalchemy#5501

Patch Set B (2/2): core files

This PR is a fork of Patch Set A (1/2), and deprecates the main library operators in favor of the newly standardized names

Fixes: sqlalchemy#5429
Change-Id: Ia1e66e7a50ac35d3f6260d8bf6ba3ce8087cbad2
jvanasco added a commit to jvanasco/sqlalchemy that referenced this issue Sep 1, 2020
@jvanasco
Issue sqlalchemy#5429 version 2: Patch Set B: core library
dc7d326 
Revised PR of sqlalchemy#5501

Patch Set B (2/2): core files

This PR is a fork of Patch Set A (1/2), and deprecates the main library operators in favor of the newly standardized names

Fixes: sqlalchemy#5429
Change-Id: Ia1e66e7a50ac35d3f6260d8bf6ba3ce8087cbad2
@jvanasco jvanasco mentioned this issue Sep 1, 2020
Closed
3 tasks
jvanasco added a commit to jvanasco/sqlalchemy that referenced this issue Oct 28, 2020
@jvanasco
while working on sqlalchemy#5435, I found some misses from the previo…
05cf77d 
…us PR for sqlalchemy#5429

Change-Id: I0be15f6234c74302734672450a3275add762bdb8
@jvanasco jvanasco mentioned this issue Oct 28, 2020
Closed
jvanasco added a commit to jvanasco/sqlalchemy that referenced this issue Oct 30, 2020
@jvanasco
while working on sqlalchemy#5435, I found some misses from the previo…
580657f 
…us PR for sqlalchemy#5429

Change-Id: I0be15f6234c74302734672450a3275add762bdb8
sqlalchemy-bot pushed a commit that referenced this issue Oct 31, 2020
@zzzeek
Merge "while working on #5435, I found some misses from the previous …
b159d09 
…PR for #5429"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jvanasco jvanasco

Labels
sql task
Projects
None yet
Milestone
1.4
5 participants
@xarg @zzzeek @jvanasco @sqla-tester @CaselIT