lucjon / Py-StackExchange

A Python binding for the StackExchange API
http://stackapps.com/questions/198/py-stackexchange-an-api-wrapper-for-python
BSD 3-Clause "New" or "Revised" License
228 stars 76 forks source link

Fix missing modules in setup.py; make iteration less broken on result se... #37

Closed aleaxit closed 9 years ago

aleaxit commented 9 years ago

...ts

(still something wrong with it -- gives duplicates for odd-sized pages -- but at least it seems to work better for page size 100).

lucjon commented 9 years ago

Hello,

Thanks very much for spotting that first issue. It's a very recent change that caused that, and I was far too hasty in merging a branch than I should have been. Thankfully no releases have been made while this fault was present!

Similar thanks are due in the case of the second issue, which is altogether quite different; far from being a recent bug, I am amazed it has escaped attention for so long. The fix is much appreciated. I have added a test case for a call similar to that described in your StackApps answer as indicated above (ignoring the clumsy commit message), which tests values for pagesize of 37 and 100 on the kruskals-algorithm tag, which has 479 questions as I write this. It confirms that the result is as expected with pagesize = 100.

The test does fail (as expected given your observations) for pagesize = 37. I will do some more digging into this soon.

aleaxit commented 9 years ago

Hi Lucas!

On Sun, Mar 15, 2015 at 7:25 PM, Lucas Jones notifications@github.com wrote:

Hello,

Thanks very much for spotting that first issue. It's a very recent change that caused that, and I was far too hasty in merging a branch than I should have been. Thankfully no releases have been made while this fault was present!

Similar thanks are due in the case of the second issue, which is altogether quite different; far from being a recent bug, I am amazed it has escaped attention for so long. The fix is much appreciated. I have added a test case for a call similar to that described in your StackApps answer http://stackapps.com/a/6218/570 as indicated above (ignoring the clumsy commit message), which tests values for pagesize of 37 and 100 on the kruskals-algorithm http://stackoverflow.com/search?q=kruskals-algorithm tag, which has 479 questions as I write this. It confirms that the result is as expected with pagesize = 100.

The test does fail (as expected given your observations) for pagesize = 37. I will do some more digging into this soon.

Peculiarly, the duplicates for pagesize 37 have disappeared from under me as I was trying to dig deeper -- gotta hate heisenbugs!-) Good to hear you can still reproduce it...!

Thanks,

Alex

— Reply to this email directly or view it on GitHub https://github.com/lucjon/Py-StackExchange/pull/37#issuecomment-81352863 .

lucjon commented 9 years ago

In fact, looking again, I just managed to mess up the first change I made to the test! There don't appear to be any duplicates in the results for 37. If you come across it again, please do pass along what triggered it.