seckcoder / prettytable

Automatically exported from code.google.com/p/prettytable
Other
0 stars 0 forks source link

Sort rows before slice. #29

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
In _get_rows, a slice occurs before any sorting takes place. I expected the 
slice to occur after. I am requesting a feature that forces the sort to happen 
before the slice.

Original issue reported on code.google.com by chao...@minorcrash.com on 3 May 2013 at 7:57

GoogleCodeExporter commented 9 years ago
I am unsure as to how to change the type to Enhancement.

Original comment by chao...@minorcrash.com on 3 May 2013 at 7:58

GoogleCodeExporter commented 9 years ago
I've changed the type to Enhancement.

Let me think a little about what the correct behaviour should be...

Original comment by luke@maurits.id.au on 4 May 2013 at 5:55

GoogleCodeExporter commented 9 years ago
The current default behavior should probably be preserved for backwards 
compatibility purposes. However, an option to override that behavior would be 
awesome.

Original comment by chao...@minorcrash.com on 10 May 2013 at 7:59

GoogleCodeExporter commented 9 years ago
Could you let me know what your use case is for wanting to sort and then slice? 
 Are you maybe trying to print like a top 10 table or something?

Original comment by luke@maurits.id.au on 10 May 2013 at 8:05

GoogleCodeExporter commented 9 years ago
Correct. The flow is such that sorting it via the table object and not the rows 
themselves is far far drier. Essentially I have several modules that produce 
tables, the user elects which of those modules is used to produce a report. The 
user can then elect to override default sorting and slicing that may have been 
specified in the module. https://github.com/chao-mu/Oopa

Original comment by chao...@minorcrash.com on 12 May 2013 at 11:38

GoogleCodeExporter commented 9 years ago
Thanks for the link to your source!

The reason I ask is that I'm grappling with the question of to what extent it's 
"right" for PrettyTable to handle what is basically manipulation of data 
(sorting, slicing, etc.) as opposed to "doing one thing well" and just printing 
what it is given, offloading the responsibility for data manipulation to 
whatever comes before it in the pipeline.

Of course, in the early days I never really gave this any thought, which is why 
sorting is in there at all.  So now I kind of feel committed to keeping them in 
and making them work "properly", even though I can forsee it requiring various 
maintenance needs down the road...

Original comment by luke@maurits.id.au on 12 May 2013 at 11:49

GoogleCodeExporter commented 9 years ago
To be perfectly honest, I have had the same thought and even discussed it with 
a friend of mine. Keeping it about presentation is architecturally the right 
way to go, for sure. Essentially, you want to separate presentation logic from 
data manipulation. What follows is a thought dump.

Maybe create a subclass that gives PrettyTable super powers. 

Or perhaps subclass "list" with a Rows subclass. People who need the 
manipulation can just use an instance of Rows or Row. Perhaps add an optional 
argument rows_class and make the rows accessible to the user to manipulate 
directly.

On that note, if a table is a series of rows, maybe just inherit from that Rows 
list subclass.

Alternatively, deprecate the existing methods using something like 
http://code.activestate.com/recipes/391367-deprecated/ and remove in the 
release after.

Or abandon this project and start a new one called PrettierTables ;-)

Maybe rename the class to DeprecatedPrettyTables and write a new one so people 
still have access to the old behavior if their code breaks. This sounds like a 
bad idea.

Just please use @property this time instead of _set/_get :-p. And maybe put the 
validation in @property_name.setter The code gets very long and confusing in 
places. That might just be a stylistic difference between us though. 

Do you have an idea of the size of your user base?

Personally, I would just break backwards compatibility and simplify everything, 
but I am a cruel coder.

Original comment by chao...@minorcrash.com on 13 May 2013 at 2:13

GoogleCodeExporter commented 9 years ago
Regarding @property - I have *always* thought that the 
get/set/property/validation code in PrettyTable was shamefully horrible, but I 
really didn't know how else to get the same functionality.  I (somehow) never 
learned about the @property decorator.  I agree that it is nicer and I will 
definitely convert in the future!  Thank you so much for this.

However, I will keep the validators separated out as individual methods instead 
of putting that code inline in the setters because (i) sometimes identical 
validators are used for different properties and code duplication is bad; (ii) 
the validators are also used at the time get_string etc. are called, because 
options can be passed in there which override any values which were set with 
the property setter.

Regading user base: it is surprisingly large.  New releases ususally rack up 
tens of thousands of downloads from PyPi in the first few days.  I have done 
one deliberate big compatibility break for the sake of simplification in the 
past (and tried to document it extensively and prominently), and I have also 
made tiny little changes that I thought nobody would notice.  Almost every time 
it causes confusion or problems or complaints for somebody.  There are also 
other projects out there which have PrettyTable as a dependency, and if I make 
backward compatibility breaking changes in PrettyTable then people using 
rolling release distros can end up with breakage in those other projects.  So I 
now take backward compatibility very seriously and if I ever need to do it 
again I will do it with deprecation notices for at least two releases.

I think for now what I will do is this: I'll make sorting/slicing work the way 
you want, because I agree it makes sense: wanting to sort *all* the rows in a 
table and then list only the top/bottom 10 is a much more reasonable case than 
wanting to take the top/bottom 10 rows *as entered* and then sort that 
arbitrary subset.  But I will put an option in somewhere to go back to the old 
behaviour in case somebody out there is somehow relying on it.  I'll try to get 
this done this evening after work.

Original comment by luke@maurits.id.au on 13 May 2013 at 4:17

GoogleCodeExporter commented 9 years ago
Okay, I just did a commit to trunk which swaps the order of slicing and 
sorting, and adds a "oldsortslice" option which gives enables the old behaviour.

Original comment by luke@maurits.id.au on 14 May 2013 at 5:46

GoogleCodeExporter commented 9 years ago
Thank you!

Original comment by chao...@minorcrash.com on 15 May 2013 at 3:16

GoogleCodeExporter commented 9 years ago

Original comment by luke@maurits.id.au on 5 Oct 2013 at 10:32