astropy / astropy

Astronomy and astrophysics core library
https://www.astropy.org
BSD 3-Clause "New" or "Revised" License
4.46k stars 1.78k forks source link

Rename QTable? #3379

Closed astrofrog closed 8 years ago

astrofrog commented 9 years ago

@taldcroft @eteq @mhvk - just realized that QTable is also the name of a class used in Qt:

http://doc.qt.digia.com/3.3/qtable.html

This could cause confusion in future. Can we consider doing a last-minute rename to QuantityTable? Yes, it's more characters, but it's also a lot more explicit.

astrofrog commented 9 years ago

(milestoning as 1.0 so we can make sure we discuss it before 1.0 final)

taldcroft commented 9 years ago

:-1: I think the namespace overlap here is not a problem because these classes occupy such very different conceptual space. In other words there would be no question when looking at a bit of code which kind of object was being created.

And of course Python has module namespacing for exactly this kind of situation. We already have overlapping names like vstack and hstack, which are probably worse examples because they are operating on similar objects with a similar API.

taldcroft commented 9 years ago

Also, the tutorials I've seen for PyQT4 all start with from PyQt4 import QtGui as the basic access idiom for all the top-level QT4 widgets. So then one would use QtGui.QTable.

astrofrog commented 9 years ago

I wouldn't be so sure about the different spaces - imagine that we someday develop a Qt based table viewer in Astropy or an affiliated package, it might start to become confusing because QTable means something specific in Qt. For vstack and hstack, the ones in astropy and numpy do conceptually the same thing. But here Q means quantity, which is completely different to Qt. So in that sense users importing QTable might think they are getting something related to Qt (because it's typical of the Qt naming convention).

astrofrog commented 9 years ago

At the end of the day, I just wanted to raise this option to also say that QTable is in itself not a great name because not very explicit, but I won't argue the point more if people are opposed to a change :)

taldcroft commented 9 years ago

It's just that in this case, QTable is something that you are actually going to type and use in code all the time, so conciseness is somewhat important. I already do QT<tab> all the time, which turns out to be very easy to type on a QWERTY keyboard. Instead it would probably become Qu<tab>T<tab>, which is actually hard because of the shift key and layout. I also find Quantity a very slow and error prone word to type in the editor, again because of the QWERTY layout.

Somehow for me going the whole way and calling it a QuantityTable gives the impression that it only holds quantities.

However, if a majority of others disagree then I would defer.

astrofrog commented 9 years ago

Ok, I'm fine with leaving things as-is, though maybe we can leave this issue open for another day or so in case anyone else has strong opinions.

pllim commented 9 years ago

I have seen usage of from PyQt4.QtGui import *. In that case, QTable might cause confusion and conflict. I am for changing it but it's not critical.

embray commented 9 years ago

I'm in favor of QuantityTable (the name, not the class--I never liked the class to begin with). I don't like "QTable" though. If I saw that I would think "What on earth is a 'Q'?"

pllim commented 9 years ago

Q is http://en.wikipedia.org/wiki/Q_%28Star_Trek%29

astrofrog commented 9 years ago

Oh, I thought it was http://en.wikipedia.org/wiki/Q_(James_Bond)

embray commented 9 years ago

And I don't think 3 keystrokes vs. 2 is a good argument against naming clarity.

astrofrog commented 9 years ago

As a side note, if we did go with a longer name, there's nothing to stop:

from astropy.table import QuantityTable as QTable
taldcroft commented 9 years ago

@embray - if you saw QuantityTable would you be at all surprised that it can contain any kind of mixin + regular columns, not just Quantities? That's my concern.

Auto-complete aside, this change goes from 6 characters to 13.

And doing the as QTable import would be missing the point because then the code still has QTable.

Having said all this, it appears I'm getting lonely here in support of QTable. Sigh.

taldcroft commented 9 years ago

BTW, I do like the subtle reference to the star trek Q, as this version of table is rather powerful!

astrofrog commented 9 years ago

So just to clarify, at the end of the day I (personally) don't mind using QTable. But you have to admit that QTable isn't any clearer a name than QuantityTable (it's just a short version). If a user sees either of those they won't get the full picture. Really, a better name would be AutoQuantityTable because that describes a little more what it does, but then it's too long a name. I'm not saying that QuantityTable is the right name, but was more interested in whether anyone had any better suggestions.

mhvk commented 9 years ago

I fear I'd be one to use from astropy.table import WhateverIsTheFinalNameTable as Table since Table is really the most logical name. However, since we are stuck with the old one, and since the main benefit of the new one is mostly that it can deal with all astropy objects (including quantities), how about just calling it AstropyTable?

taldcroft commented 9 years ago

My feeling is that a really explicit name is not always needed for core elements that appear all the time and you use a dozen times a day. Many of the classes we use the most often are really generic overloaded words that don't really tell you much about what's really inside.

But since none of the other devs appears to like QTable then maybe the user community won't either.

@mhvk - Table can handle all other astropy objects except Quantity. I would prefer QuantityTable to AstropyTable.

astrofrog commented 9 years ago

Just to put out a crazy suggestion out there (you know, just to see), maybe we could in fact disable mix-in columns from the main Table class and then rename QTable to MixinTable. That way we have a separation between the simple Table class that supports data as before, and this new cool table that supports all the fancy mix-in stuff. The advantage of that is that it's 'all or nothing' so there's not a weird in between state where SkyCoord is supported and Quantity isn't. So then there's a real incentive for people to use MixinTable (for lack of a better name).

astrofrog commented 9 years ago

(I mentioned this before, but if we went with my last suggestion above, I think that SmartTable would be a good name - sure it doesn't exactly explain what it is either, but it sounds cool to users!)

taldcroft commented 9 years ago

But then you can't have a MixinTable without Quantities, so then you need a third one which is MixinTableWithNoQuantity, which is in fact the table class that I would use typically. I'm not too keen on that.

embray commented 9 years ago

if you saw QuantityTable would you be at all surprised that it can contain any kind of mixin + regular columns, not just Quantities? That's my concern.

I would, but that's one of the reasons I don't like it in the first place :) That's not a fight I've really had the time to invest in though, and I know it made some people happy so fine. QTable is no less confusing in that regard though.

embray commented 9 years ago

BTW, I do like the subtle reference to the star trek Q, as this version of table is rather powerful!

:+1: Maybe it can be available as an alias? ;)

astrofrog commented 9 years ago

@taldcroft - just curious, why would you want MixinTableWithNoQuantity? So you don't actually plan on using QTable?

astrofrog commented 9 years ago

The reason I'm asking this is that if MixinTableWithNoQuantity is what you would use typically (aka the current Table), why do you mind if the QTable name is longer? :wink:

taldcroft commented 9 years ago

As a table developer I care intensely about the user experience, and project my own preferences onto users. Also as a table developer I find myself typing QTable quite often, probably more often than most users ever will.

Over the next year or two in my own day-to-day work I will mostly use Table because I have lots of legacy code that uses various table inputs that would pretty much break if all the columns with units suddenly became Quantity. I am, however, excited about using Time in tables for my work because that will be a small change for a pretty good improvement in the way the code works. But I do imagine that going forward as I write new tools or packages that I might wind up using QTable.

mhvk commented 9 years ago

For the reasons of confusion mentioned above by others, I don't like QuantityTable as it suggests a specificity that is not there. If stuck between that and QTable, I'd prefer the latter because it is less clear about what it it. But better might be a generic name like SmartTable or AstropyTable (where the latter would be short for FullyAstropyCompatibleTable :smile: -- maybe FACTable or ATable...). ... ... ... ... OK, too much time spent on nomenclature!

taldcroft commented 9 years ago

Anyway, shouldn't we have had this discussion before #3011 was merged? :wink:

astrofrog commented 9 years ago

@taldcroft - yes, and really the only point I wanted to make here is a clash with Qt that I only became aware of today (hence why I didn't bring it up before #3011).

In any case, at the end of the day, there's no consensus on what a better option would be, and as @mhvk pointed out, the fact QTable is not obvious at first glance will force users to RTFD, so in that sense I'm fine with sticking to it. I still think it will complicate things a little when dealing with Qt, but I guess I'm in the minority. If users ever bring up the confusion, I'll at least have the satisfaction of being able to say "I told you so" :wink:

Seriously though, if we ever decide it's a really bad name and someone has an excellent suggestion, maintaining backward compatibility to the old name would be easy.

embray commented 9 years ago

If I had the time I would try to think about a better way of interacting with tables such that it wouldn't require a special class for this purpose. I think such an interface exists (I've even put out a few suggestions before but none were quite on the mark, and I haven't had time to develop them). Meanwhile other smart people have thought long and hard about this so it's presumptuous of me to think we can find anything better. To me at just makes sense to have a Column that represents how some data (whatever it is represented as) is stored in a table, and the data itself separate. And I'm happy to just do table['colname'].data if I want a Quantity or whatever. But that's just me.

astrofrog commented 9 years ago

No consensus, so removing the milestone and leaving open a little, will close if there is no more discussion.

eteq commented 9 years ago

My reading of this thread is that there's no consensus on a better alternative... so should we just close this and stuck with the status quo?

(FWIW, I have no strong opinion - while in general I favor better to more explicit, I want to use QTable only going forward, and I do like the shorter name. And the probability of name collision is pretty low...)

eteq commented 9 years ago

hah, @astrofrog beat me to it by 15 seconds ;)

embray commented 9 years ago

shifts uncomfortably in seat I still don't like it, but I can't offer a better alternative for now either. The good news is that I don't think it's something to worry about being locked into--if some amazing perfect alternative does emerge the QTable implementation could still likely be supported. I'm for leaving this open for now, but deferred.

eteq commented 9 years ago

I milestoned it "future", which I believe is code for "maybe never"? ;)

taldcroft commented 9 years ago

The QTable class is relieved, having survived a last-minute bid to end its existence.

hamogu commented 8 years ago

Since QTable was released and we generally want API stability and there where no further comments on this in ~1 year, I suggest to close it. If we kept an open issue for anything that somebody suggested to rename in the past we we decided to not rename it,we'd probably overflow the possible number of open issues!

taldcroft commented 8 years ago

@hamogu - I agree. I'm closing this now.

@astrofrog @eteq @mhvk - feel free to re-open if you feel it necessary.