www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - std.bitmanip improvements

reply "Marco Nembrini" <marco.nembrini.co gmail.com> writes:
Hi,
I was looking into the BitArray part of std.bitmanip to implement 
various data compression routines like Huffmann, Lempel-Ziv etc.. 
and noticed that it could use some improvements, like more 
convenient constructors and functions (for ex. see issue 4717 ). 
How would I go about submitting those changes? Should I put all 
the changes in one big pull request or split it up according to 
the different enhancement requests on Bugzilla?
Also is anyone else working on this module or is there something 
in other parts of the library that I should be aware of?

Cheers,
Marco
Dec 27 2012
next sibling parent "bearophile" <bearophileHUGS lycos.com> writes:
Marco Nembrini:

 Also is anyone else working on this module or

Recently I remember some discussions in D.learn about someone working on that array of bits. Bye, bearophile
Dec 27 2012
prev sibling next sibling parent "Daniel Murphy" <yebblies nospamgmail.com> writes:
"Marco Nembrini" <marco.nembrini.co gmail.com> wrote in message 
news:ezcalrajgcqlspefnlri forum.dlang.org...
 Should I put all the changes in one big pull request or split it up 
 according to the different enhancement requests on Bugzilla?

Split it up.
Dec 27 2012
prev sibling next sibling parent reply Dmitry Olshansky <dmitry.olsh gmail.com> writes:
12/27/2012 1:38 PM, Marco Nembrini пишет:
 Hi,
 I was looking into the BitArray part of std.bitmanip to implement
 various data compression routines like Huffmann, Lempel-Ziv etc.. and
 noticed that it could use some improvements, like more convenient
 constructors and functions (for ex. see issue 4717 ). How would I go
 about submitting those changes? Should I put all the changes in one big
 pull request or split it up according to the different enhancement
 requests on Bugzilla?

Judging by my experience small pulls that have no extra dependencies on other pulls are the fastest to get merged. The are also more throughly reviewed :) So I'd suggest do it step by step in a couple of pulls if possible.
 Also is anyone else working on this module or is there something in
 other parts of the library that I should be aware of?

IRC Era Scarecrow did an amazing job on it that covered a lot of enhancements including small string optimization, appending, slicing etc. I recall reviewing his code and it was a Phobos pull once but then it pulled out (for further enhancement? dunno). So definitely suggest you two to get in touch.
 Cheers,
 Marco

-- Dmitry Olshansky
Dec 27 2012
next sibling parent reply Marco Nembrini <marco.nembrini.co gmail.com> writes:
On 28.12.2012 11:48, Era Scarecrow wrote:
 On Thursday, 27 December 2012 at 10:16:51 UTC, Dmitry Olshansky wrote:
 IRC Era Scarecrow did an amazing job on it that covered a lot of
 enhancements including small string optimization, appending, slicing etc.

 I recall reviewing his code and it was a Phobos pull once but then it
 pulled out (for further enhancement? dunno). So definitely suggest you
 two to get in touch.

I'm glad for the praise... :) Perhaps it seems silly, however I pulled it not because I needed to enhance it, but because it had trouble merging with other pulls. I looked at the error(s) and it was something like '13 white spaces mismatches'... How do you fix 13 white spaces? They're white space because you can't see them :P Had it been lines of code you can intelligently re-order or adjust them, but white spaces?? Sheesh... Maybe being lazy but it was a headache trying to figure out the diff patch and how to fix it, so haven't tried too hard on it yet. If someone happens across that I'll give it another go and see if there's any other fixes/enhancements I can throw on it; Maybe get it in before the next release.

So the code was complete and working and the only problem was doing the pull on github? If so would you mind me trying to do that in your place? It seems worth spending some time on to get all those improvements in. Is the latest version this one: https://github.com/rtcvb32/phobos ?
Dec 27 2012
parent Marco Nembrini <marco.nembrini.co gmail.com> writes:
On 01.01.2013 19:14, Era Scarecrow wrote:
 On Friday, 28 December 2012 at 01:21:01 UTC, Marco Nembrini wrote:
 If so would you mind me trying to do that in your place? It seems
 worth spending some time on to get all those improvements in.

Well since this is going I've resumed some work on it, nice to see the opBinary works along with the opDollar. I seem to have problems figuring out where to tell my github source to get the current beta so I can just work/update from that. All of it is quite confusing to me.

Checkout the section about rebasing here: http://wiki.dlang.org/Pull_Requests . If you still have trouble send me a message, I should be able to help with git at least. Cheers -- Marco Nembrini
Jan 01 2013
prev sibling parent Dmitry Olshansky <dmitry.olsh gmail.com> writes:
12/28/2012 5:13 AM, Marco Nembrini пишет:
 On Thursday, 27 December 2012 at 10:16:51 UTC, Dmitry Olshansky wrote:
 IRC Era Scarecrow did an amazing job on it that covered a lot of
 enhancements including small string optimization, appending, slicing etc.

 I recall reviewing his code and it was a Phobos pull once but then it
 pulled out (for further enhancement? dunno). So definitely suggest you
 two to get in touch.

I assume you are referring to the code in this thread: http://forum.dlang.org/thread/gtfxxmlqwgyadmpygftv forum.dlang.org ?

-- Dmitry Olshansky
Dec 27 2012
prev sibling next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Thursday, December 27, 2012 10:38:35 Marco Nembrini wrote:
 Hi,
 I was looking into the BitArray part of std.bitmanip to implement
 various data compression routines like Huffmann, Lempel-Ziv etc..
 and noticed that it could use some improvements, like more
 convenient constructors and functions (for ex. see issue 4717 ).
 How would I go about submitting those changes? Should I put all
 the changes in one big pull request or split it up according to
 the different enhancement requests on Bugzilla?
 Also is anyone else working on this module or is there something
 in other parts of the library that I should be aware of?

I believe that there are a couple of threads from a few months ago in D.Learn talking about improvements to BitArray, and some folks having been working on that. I have no idea where that stands though. In general, if you have incremental improvements for things, feel free to create pull requests for them on github. Unrelated requests should be separate. Please don't make huge pulls with lots of stuff in them. It makes reviewing them harder, and it's more problematic when some of your changes are acceptable and others aren't. If they're separate, they're much more easily managed. However, if you're talking about seriously revamping the module as a whole rather than making a number of small improvements, then that may merit a full review in the newsgroup, which is a different beast altogether. - Jonathan M Davis
Dec 27 2012
prev sibling next sibling parent "Marco Nembrini" <marco.nembrini.co gmail.com> writes:
On Thursday, 27 December 2012 at 10:21:37 UTC, Jonathan M Davis 
wrote:
 I believe that there are a couple of threads from a few months 
 ago in D.Learn
 talking about improvements to BitArray, and some folks having 
 been working on
 that. I have no idea where that stands though.

Ok, I'll try to read up on that and get in touch with others involved.
 In general, if you have incremental improvements for things, 
 feel free to
 create pull requests for them on github. Unrelated requests 
 should be
 separate. Please don't make huge pulls with lots of stuff in 
 them. It makes
 reviewing them harder, and it's more problematic when some of 
 your changes are
 acceptable and others aren't. If they're separate, they're much 
 more easily
 managed.

 However, if you're talking about seriously revamping the module 
 as a whole
 rather than making a number of small improvements, then that 
 may merit a full
 review in the newsgroup, which is a different beast altogether.

 - Jonathan M Davis

I was thinking of mostly small convenience changes, I don't feel familiar enough with D yet to do a revamp of that, but maybe I can help get Era Scarecrow's version pulled.
Dec 27 2012
prev sibling next sibling parent "monarch_dodra" <monarchdodra gmail.com> writes:
On Thursday, 27 December 2012 at 10:16:51 UTC, Dmitry Olshansky 
wrote:
 IRC Era Scarecrow did an amazing job on it that covered a lot 
 of enhancements including small string optimization, appending, 
 slicing etc.

 I recall reviewing his code and it was a Phobos pull once but 
 then it pulled out (for further enhancement? dunno). So 
 definitely suggest you two to get in touch.

Last time I was in touch with him, he was getting increasingly frustrated trying to learn to use github. I *think* (but don't take my word for it), that he may have abandoned the pull after having too much trouble trying to rebase it. In any case, I'll try to contact him directly and make him aware of this thread.
Dec 27 2012
prev sibling next sibling parent "Era Scarecrow" <rtcvb32 yahoo.com> writes:
On Thursday, 27 December 2012 at 10:16:51 UTC, Dmitry Olshansky 
wrote:
 IRC Era Scarecrow did an amazing job on it that covered a lot 
 of enhancements including small string optimization, appending, 
 slicing etc.

 I recall reviewing his code and it was a Phobos pull once but 
 then it pulled out (for further enhancement? dunno). So 
 definitely suggest you two to get in touch.

I'm glad for the praise... :) Perhaps it seems silly, however I pulled it not because I needed to enhance it, but because it had trouble merging with other pulls. I looked at the error(s) and it was something like '13 white spaces mismatches'... How do you fix 13 white spaces? They're white space because you can't see them :P Had it been lines of code you can intelligently re-order or adjust them, but white spaces?? Sheesh... Maybe being lazy but it was a headache trying to figure out the diff patch and how to fix it, so haven't tried too hard on it yet. If someone happens across that I'll give it another go and see if there's any other fixes/enhancements I can throw on it; Maybe get it in before the next release.
Dec 27 2012
prev sibling next sibling parent "Marco Nembrini" <marco.nembrini.co gmail.com> writes:
On Thursday, 27 December 2012 at 10:16:51 UTC, Dmitry Olshansky 
wrote:
 IRC Era Scarecrow did an amazing job on it that covered a lot 
 of enhancements including small string optimization, appending, 
 slicing etc.

 I recall reviewing his code and it was a Phobos pull once but 
 then it pulled out (for further enhancement? dunno). So 
 definitely suggest you two to get in touch.

I assume you are referring to the code in this thread: http://forum.dlang.org/thread/gtfxxmlqwgyadmpygftv forum.dlang.org ?
Dec 27 2012
prev sibling next sibling parent "Era Scarecrow" <rtcvb32 yahoo.com> writes:
On Friday, 28 December 2012 at 01:21:01 UTC, Marco Nembrini wrote:
 So the code was complete and working and the only problem was 
 doing the pull on github? If so would you mind me trying to do 
 that in your place? It seems worth spending some time on to get 
 all those improvements in.

In a nutshell, yes..
 Is the latest version this one: 
 https://github.com/rtcvb32/phobos ?

I'll have to look at my sources, I may have been working on something else. I can't recall but I think the settings were 'sometimes reference', although that's not hard to fix, just need to to decide the best way to handle the data that way. I'll look over all my stuff tomorrow. Feel free to try and get it to merge/pull in GitHub.
Dec 27 2012
prev sibling parent "Era Scarecrow" <rtcvb32 yahoo.com> writes:
On Friday, 28 December 2012 at 01:21:01 UTC, Marco Nembrini wrote:
 If so would you mind me trying to do that in your place? It 
 seems worth spending some time on to get all those improvements 
 in.

Well since this is going I've resumed some work on it, nice to see the opBinary works along with the opDollar. I seem to have problems figuring out where to tell my github source to get the current beta so I can just work/update from that. All of it is quite confusing to me.
Jan 01 2013