If it is a change to one of the core classes we need to make a Jira + patch.
If it is just a change to CSVFeatureWriter then you can commit directly?
--
Jody Garnett
On Monday, 16 May 2011 at 8:45 AM, lee-verizon wrote:
> The javadoc in FeatureWriter:
>
> * After aquiring a feature using next() you may call remove() or after
> * modification write(). If you do not call one of these two methods before
> * calling hasNext(), or next() for that matter, the feature will be left
> * unmodified.
>
> So yeah, I think we should change removeFeatures as shown below.
>
> I have update, remove, and add all working for CVS (with the
> removeFeatures() mod). I do need to add more tests for edge cases, but I'd
> like to commit what I have. So, I should create a JIRA for that
> removeFeautures() bug? Then after it is fixed, I can checkin the CSV code.
>
> Thanks,
>
> Lee
>
> On 5/13/2011 3:21 PM, Jody Garnett wrote:
> > I think I agree with that; my understanding is it is either write() or
> > remove()? But really we better check the javadocs - as I have known to be
> > wrong :-)
> >
> > --
> > Jody Garnett
> >
> > On Saturday, 14 May 2011 at 12:19 AM, lee-verizon wrote:
> > > Moving on to remove... I think there really is a bug in
> > > ContentFeatureStore>removeFeatures(). The main loop looks like this:
> > >
> > > while( writer.hasNext() ) {
> > > writer.next();
> > > writer.remove();
> > > writer.write();
> > > }
> > >
> > > I think that last writer.write() should not be there. Jody, you even
> > > said (on IRC):
> > >
> > > (10:49:27 PM) jgarnett: next() returns a feature
> > > (10:49:31 PM) jgarnett: for people to hack away at
> > > (10:49:38 PM) jgarnett: they can either call
> > > (10:49:39 PM) jgarnett: write()
> > > (10:49:40 PM) jgarnett: or
> > > (10:49:43 PM) jgarnett: remove()
> > >
> > > Whadya think?
> > >
> > > Lee
> > >
> > >
> > > On 5/12/2011 11:27 PM, lee-verizon wrote:
> > > > I thought of another way to do it that confines the fix to
> > > > CSVFeatureWriter and does not involve any changes to
> > > > FilteringFeatureWriter. Much cleaner, IMO.
> > > >
> > > > Still need to do remove and add...
> > > >
> > > > Lee
> > > >
> > > > On 5/12/2011 8:59 AM, lee-verizon wrote:
> > > > > ok, I came up with something that seems to work - in
> > > > > FilteringFeatureWriter>hasNext(), it is doing this:
> > > > >
> > > > > SimpleFeature peek;
> > > > > while (writer.hasNext()) {
> > > > > peek = writer.next();
> > > > > if (filter.evaluate(peek)) {
> > > > > next = peek;
> > > > > return true; // we have a match!
> > > > > }
> > > > > }
> > > > >
> > > > > I tweaked it like this:
> > > > >
> > > > > SimpleFeature peek;
> > > > > while (writer.hasNext()) {
> > > > > peek = writer.next();
> > > > > if (filter.evaluate(peek)) {
> > > > > next = peek;
> > > > > return true; // we have a match!
> > > > > } else {
> > > > > // TODO this should check the update flags
> > > > > writer.write(); // allow writer to deal with non-matched
> > > > > }
> > > > > }
> > > > >
> > > > > That fixed it for the CSV unit tests I've written, but I'm concerned
> > > > > that it will break other clients of FilteringFeatureWriter. I tried
> > > > > to run tests in other modules (e.g. jdbc-h2), but not sure I'm
> > > > > running the tests correctly.
> > > > >
> > > > > Lee
> > > > >
> > > > > On 5/11/2011 10:40 PM, lee-verizon wrote:
> > > > > > On 5/11/2011 7:36 PM, Jody Garnett wrote:
> > > > > > >
> > > > > > > On Thursday, 12 May 2011 at 10:30 AM, [email protected]
> > > > > > > wrote:
> > > > > > >
> > > > > > > > Sorry, I forgot to mention that I already did put in the
> > > > > > > > FilteringFeatureWriter and it works fine. Perhaps too fine.
> > > > > > > > ? so that part works ?
> > > > > > > Yes, worked first time. It delivers just the rows that match the
> > > > > > > filter.
> > > > > > > Or maybe it just needs more work. I like the idea of chaining
> > > > > > > > together various wrapped writers, but I just don't see how the
> > > > > > > > current set of them can work.
> > > > > > > > I may need to write you up some more of them; this is the
> > > > > > > > first time
> > > > > > > ContentDataStore is being used for a file format after all.
> > > > > > > Ah, I didn't realize this is still sortof work-in-progress. In
> > > > > > > that
> > > > > > case, I'll feel more 'free' to hack away.
> > > > > > > Can we chat sometime on IM, or is that what IRC is for? I'm
> > > > > > > > traveling at the moment but should be online later tonight
> > > > > > > > (I'm in
> > > > > > > > the US, west coast).
> > > > > > > > IRC is good (#geotools channel - details in the user guide)
> > > > > > > I think I just need to better understand the intent of the
> > > > > > FeatureWriter methods. It seems that clever combinations of
> > > > > > next(),
> > > > > > hasNext(), and write() produce various behaviors. I'll go read the
> > > > > > docs again and look at AbstractDataStore for examples.
> > > > > >
> > > > > > Lee
> > > > > >
> > > > >
> > > >
> > >
> >
>
------------------------------------------------------------------------------
Achieve unprecedented app performance and reliability
What every C/C++ and Fortran developer should know.
Learn how Intel has extended the reach of its next-generation tools
to help boost performance applications - inlcuding clusters.
http://p.sf.net/sfu/intel-dev2devmay
_______________________________________________
Geotools-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geotools-devel
