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

rename rows to all #16

Closed
chadwhitacre opened this Issue Aug 8, 2013 · 5 comments

Comments

Projects
None yet
2 participants
@chadwhitacre
Owner

chadwhitacre commented Aug 8, 2013

Our simple API: run / one / rows

one and rows aren't parallel. :-(

We should either change one to row or rows to many. I like row better because it keeps the aliteration and one and many feel ORM-ish to me.

Since we've already released 1.0.0 we've got a backwards-compatibility issue. Since we're so early in the project (I released 1.0.0 about 8 or 9 hours ago), I think we can get away with aliasing row as one but not documenting it.

@chadwhitacre

This comment has been minimized.

Show comment
Hide comment
@chadwhitacre

chadwhitacre Aug 8, 2013

Owner

Ugh, we've also got strict_one to deal with, and since the docs are autogenerated, we can't easily change that in code without mucking up the docs. :-/

Should we just change 1.0.0? :-(

Owner

chadwhitacre commented Aug 8, 2013

Ugh, we've also got strict_one to deal with, and since the docs are autogenerated, we can't easily change that in code without mucking up the docs. :-/

Should we just change 1.0.0? :-(

@chadwhitacre

This comment has been minimized.

Show comment
Hide comment
@chadwhitacre

chadwhitacre Aug 8, 2013

Owner

I think we should. :-(

Owner

chadwhitacre commented Aug 8, 2013

I think we should. :-(

@chadwhitacre

This comment has been minimized.

Show comment
Hide comment
@chadwhitacre

chadwhitacre Aug 8, 2013

Owner

Now I'm worried about row and rows being too similar, introducing bugs where you thought you were calling one but were calling the other.

for row in db.row("SELECT * FROM foo"):
   print(row)

vs.

for row in db.rows("SELECT * FROM foo"):
   print(row)

vs.

for row in db.one("SELECT * FROM foo"):
   print(row)

vs.

for row in db.many("SELECT * FROM foo"):
   print(row)

vs.

for row in db.one("SELECT * FROM foo"):
   print(row)

vs.

for row in db.rows("SELECT * FROM foo"):
   print(row)

Perhaps the cognitive obstacle between one and rows is good, because it helps you not confuse them.

I've already confused one and run. I did:

db.one("CREATE TABLE foo (bar text)")

Now that we have strict checking on one this raises postgres.TooFew: Got -1 rows instead of 1. Otherwise you get psycopg2.ProgrammingError: no results to fetch. (Hmmm ... makes me think maybe we should wrap one calls in a transaction and only commit them if there's exactly one row, because the table is still created here. Reticketed as #17.)

The confusion between one and run fails early and hard, when it fails:

  • If you call one when you meant to call run:
    • The query doesn't return one row and you get an exception (assuming you're using strict one checking—perhaps a good reason to).
    • The query does return one row and you have simply discarded it: not a problem.
  • If you call run when you meant to call one then you get None back, which if you are using strict one checking will be a surprise—another good reason to use that. Since you'd otherwise be expecting a dict (assuming you're using the default of RealDictCursor) then the failure would probably manifest as TypeError: 'NoneType' object is not subscriptable when you tried to do row['foo'].

By contrast if you were to call row when you meant rows you would get a dict that you could still iterate over. The bug would then manifest when you tried to treat one of the scalar dict values like a dict. If you call rows when you meant row then you'd get a list where you expect a dict and you'd probably see TypeError: list indices must be integers, not str.

In other words, strict one checking makes it less harmful to confuse one and run than it is to confuse one and rows. Should we rename rows to many? That feels too ORM-y to me. For me it invokes many-to-many relationships and relational algebra, which is decidedly not what we're modeling in Python here. rows is much more matter-of-fact, and fits better with the common case where you do:

for row in db.rows(SELECT, params):
    print(row)

But geez, this whole thing started this morning because I found myself wanting to say run / one / many. I don't like all there because my vim mis-highlights it as a builtin. :-/ It's also three letters instead of four which makes it easier to confuse with one (as well as run, I suppose). many fits fine semantically and it's longer than one so that's a subtle hint.

for row in db.many(SELECT, params):
    print(row)

I don't want to get into a holy war over ORMs so I shouldn't care about that factor. The question is what's going to be the most obvious API here and while there's probably not a "right" answer there's righter answers than others. I know it's annoying in CSS that it's vertical-align and text-align ... why isn't it horizontal-align? It should be parallel. If the goal is not to confuse them then one and many are certainly better than row and rows. Also, since we don't have to worry about the strict_one problem then we could simply alias rows = many and call it a day. I'm slightly annoyed that many could return zero or one result, which technically aren't many. I guess I can tell myself that it's an announcement of willingness to receive: "I'm willing to receive one result. I'm willing to receive many results." Meh. Don't like it. It could also return 10 out of 100 possible rows and still be many when we really want all rows, guaranteed. all really fits better semantically, but three characters, too similar. The vim thing can't be a reason.

Has this cave pinched out yet?

Owner

chadwhitacre commented Aug 8, 2013

Now I'm worried about row and rows being too similar, introducing bugs where you thought you were calling one but were calling the other.

for row in db.row("SELECT * FROM foo"):
   print(row)

vs.

for row in db.rows("SELECT * FROM foo"):
   print(row)

vs.

for row in db.one("SELECT * FROM foo"):
   print(row)

vs.

for row in db.many("SELECT * FROM foo"):
   print(row)

vs.

for row in db.one("SELECT * FROM foo"):
   print(row)

vs.

for row in db.rows("SELECT * FROM foo"):
   print(row)

Perhaps the cognitive obstacle between one and rows is good, because it helps you not confuse them.

I've already confused one and run. I did:

db.one("CREATE TABLE foo (bar text)")

Now that we have strict checking on one this raises postgres.TooFew: Got -1 rows instead of 1. Otherwise you get psycopg2.ProgrammingError: no results to fetch. (Hmmm ... makes me think maybe we should wrap one calls in a transaction and only commit them if there's exactly one row, because the table is still created here. Reticketed as #17.)

The confusion between one and run fails early and hard, when it fails:

  • If you call one when you meant to call run:
    • The query doesn't return one row and you get an exception (assuming you're using strict one checking—perhaps a good reason to).
    • The query does return one row and you have simply discarded it: not a problem.
  • If you call run when you meant to call one then you get None back, which if you are using strict one checking will be a surprise—another good reason to use that. Since you'd otherwise be expecting a dict (assuming you're using the default of RealDictCursor) then the failure would probably manifest as TypeError: 'NoneType' object is not subscriptable when you tried to do row['foo'].

By contrast if you were to call row when you meant rows you would get a dict that you could still iterate over. The bug would then manifest when you tried to treat one of the scalar dict values like a dict. If you call rows when you meant row then you'd get a list where you expect a dict and you'd probably see TypeError: list indices must be integers, not str.

In other words, strict one checking makes it less harmful to confuse one and run than it is to confuse one and rows. Should we rename rows to many? That feels too ORM-y to me. For me it invokes many-to-many relationships and relational algebra, which is decidedly not what we're modeling in Python here. rows is much more matter-of-fact, and fits better with the common case where you do:

for row in db.rows(SELECT, params):
    print(row)

But geez, this whole thing started this morning because I found myself wanting to say run / one / many. I don't like all there because my vim mis-highlights it as a builtin. :-/ It's also three letters instead of four which makes it easier to confuse with one (as well as run, I suppose). many fits fine semantically and it's longer than one so that's a subtle hint.

for row in db.many(SELECT, params):
    print(row)

I don't want to get into a holy war over ORMs so I shouldn't care about that factor. The question is what's going to be the most obvious API here and while there's probably not a "right" answer there's righter answers than others. I know it's annoying in CSS that it's vertical-align and text-align ... why isn't it horizontal-align? It should be parallel. If the goal is not to confuse them then one and many are certainly better than row and rows. Also, since we don't have to worry about the strict_one problem then we could simply alias rows = many and call it a day. I'm slightly annoyed that many could return zero or one result, which technically aren't many. I guess I can tell myself that it's an announcement of willingness to receive: "I'm willing to receive one result. I'm willing to receive many results." Meh. Don't like it. It could also return 10 out of 100 possible rows and still be many when we really want all rows, guaranteed. all really fits better semantically, but three characters, too similar. The vim thing can't be a reason.

Has this cave pinched out yet?

@chadwhitacre

This comment has been minimized.

Show comment
Hide comment
@chadwhitacre

chadwhitacre Aug 8, 2013

Owner

Reopened for @tshepang.

We could use all with a simple alias. We do advertise as being "an abstraction over psycopg2" so maintaining the association with DB-API 2.0 is relevant.

Owner

chadwhitacre commented Aug 8, 2013

Reopened for @tshepang.

We could use all with a simple alias. We do advertise as being "an abstraction over psycopg2" so maintaining the association with DB-API 2.0 is relevant.

chadwhitacre added a commit that referenced this issue Aug 8, 2013

@tshepang

This comment has been minimized.

Show comment
Hide comment
@tshepang

tshepang Aug 8, 2013

Contributor

Sorry, it was a busy day here at work, so I didn't get a chance to visit this site, but I am quite satisfied with the choice you made... one / all. I am also glad that you took the time to write such detailed thoughts.

Contributor

tshepang commented Aug 8, 2013

Sorry, it was a busy day here at work, so I didn't get a chance to visit this site, but I am quite satisfied with the choice you made... one / all. I am also glad that you took the time to write such detailed thoughts.

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