[QGIS-Developer] Temporal controller issues

[Cory Albrecht]

>  and that using it for data analysis has been a secondary goal


I'm not doing any data analysis. In fact, I doubt that I use even 10% of
QGIS's capability.


I draw a feature that is the Kingdom of France and set the start_date and
end_date attributes appropriately (with yyyy-mm-dd resolution) and set a
few other features (colour, default name, government type, etc...). Then I
copy that feature, move the time frame to the day after end_date, paste,
and edit the attributes of that new feature as the Second French Republic.
Then again as the Second French Empire, then the Third Republic, and so on.
If I make an error, like somehow I didn't get the border with Belgium quite
right I will use either the Vertex Tool (from the Digitizing Toolbar) or
the Reshape Features Tool (from the Advanced Digitizing Toolbar), depending
on the scale of the edit. If that feature that I am pasting has its borders
change (like when the Second French Empire lost Alsace to the German
Empire), I will use the Split Features Tool to carve off a chunk to merge
with that other feature.


That's like 90% of my QGIS workflow right there, the drawing and simple
editing of features. The rest is creating print layouts for exporting to
SVG to work on artistic renderings. Nothing fancy and no data analysis.
It's a simple data entry workflow.


> 1. The temporal framework was in discussion and consultation phases

for years prior to implementation.


Then how did something as simple as the selection tool selecting stuff that
is not there get past this? Did nobody say "Hey, we need to make sure that
when the NTC is turned on that other parts of the application work as
expected"?


For example, one can add all sorts of SQL-like filters to a layer
datasource (which was how the TimeManager plugin worked, if I understand
correctly) and the selection tool, vertex tool, and so on are not broken by
that. When somebody turns on the NTC, it is reasonable for them to expect
that the filtering it does would act in a similar manner to those SQL-like
filters added manually to a layer, yes?


This is why in my initial email I asked about the pipeline from retrieval
of features in the layer's data provider to drawing them on the canvas.
Prior to the NTC, did tools like selection, vertex, identify, and so on
work off of the same "root" list of features that the map canvas drawer
worked from? Because these bugs suggest an implementation that either
creates a second, filtered list of features and the canvas draws gets
passed that list instead of the "root" list while the tools still work off
of the "root" list, or that the canvas drawing code was modified to to do
the temporal filtering itself and draw only a subset of the "root" list
passed to it.


You’ve said that you had to fix the bugs I reported separately, but why? To
my mind, the NTC should be filtering that "root" list of features, and then
nothing else needs to be modified. Not the code that calls the canvas
drawer method, not the code for the selection tool. Not the code of the
identify feature tool. Don’t change the tools to be time-aware
individually, keep them time-agnostic and change the dataset that gets
passed to them, and implement that in a centralized location. Then later
when somebody with that rare use case of needing to select even the
filtered-out features asks for it, you can modify that centralized location
to return the full dataset, not the temporally filtered one if certain
conditions are met, like an app setting or using the Alt key when clicking
on the canvas, or whatever, and it will work for all tools, not just one.


(I’m willing to bet you a $100 donation that usage cases that need the
selection tool to work on all the features including those filtered out by
the NTC and not drawn is tiny to nonexistent, compared to the one where the
tool is expected to work in a WYSIWYG manner only on the visible features.
If I am wrong, I'll double that donation.)


However, since I do not know the codebase, I asked for that help in
understanding how the NTC was implemented and why that way, due to other
considerations of which I would be unaware that prevented that. It was also
why I phrased my final line of criticism as “I feel” rather than as a
statement of fact.


> 2. QGIS already has some of the most stringent and rigorous code review
processes around, and the pull requests implementing the temporal framework
were subject to all these processes and peer review.


Then how did this problem get past that review? Did nobody take the feature
branch, compile it, then run QGIS and do some common, simple tasks
regarding data entry with the NTC turned on to see that those tools are
working on the “root” layer data and not the filtered data one sees on the
map canvas? (Or an automated GUI test harness that does the same thing?) Or
was it only tested by loading a project with temporal layers and changing
the viewing time frame and seeing that only matching features were drawn on
the map canvas?


> 3. If you're asking for the pace of feature development to be slowed

then that problem was addressed years ago when the LTR releases were

introduced.


No, not at all. That was purely a semantics comment about not using
standard terminology, which makes things confusing.


> The QGIS website is quite clear in advertising the LTR as the officially
recommended version for stability and for mission critical work, while the
non-LTR releases are clearly branded as "bleeding edge".


I would disagree about that clarity. When you go to the main page, for
example, there's just a download button with text underneath saying
"Version 3.16.2 Version 3.10.13 LTR". On the download page it just says
"riches on features" and "most stable". And the user manual for 3.16 itself
says "We recommend that you use this version over previous releases."


I couldn't find anything with the qgis.org search feature that says what
you say here.


> I'd make the case that in the situation you've described you should never
have been using the non-LTR release


Unfortunately I've been somewhat forced into using master because other
bugs I have reported about how QGIS handles array fields in PostGIS layers.
Without those fixes I sometimes couldn't even work on my maps because I
couldn't save features without data corruption, or sometimes not even save
at all. Those fixes, when they finally got done (and some still aren't),
took a while to end up in the LTR.


> So I ask again, in the full spirit of reconciliation and moving forward:
what concrete, practical changes do YOU think the project should make?


Testing, like I mentioned above. Better testing.


The NTC affects what gets displayed on the map canvas, therefore other
tools that operate on features on the map canvas should have been tested to
see how the interacted with the NTC. "Does my new capability hide features
from the map canvas? I should test the tools that one can use on features
on the the map canvas to make sure it doesn't operate on the features
hidden by my new capability.” Do you see how it looks like that type of
testing was not done before the branch with the NTC got merged with master?

On Tue, Jan 5, 2021 at 5:00 AM Nyall Dawson <nyall.dawson at gmail.com> wrote:

> On Tue, 5 Jan 2021 at 05:42, Cory Albrecht <maps at hanfastolfe.com> wrote:
> >
> > > Please be very wary of your language in future -- every piece of
> feedback worded like this directly equates to a developer losing interest
> in volunteering their time on QGIS, to the harm of all.
> >
> > I hear what you're saying, Nyall, I apologise if it sounded harsh, but
> what do you want me to say after I find out that the latest of several bugs
> has compromised my data and now I have to hunt down an unknown number of
> duplicates silently created over the last few months of work? If you
> empathise, do you understand how all these bugs and my data being
> compromised might make me feel that this feature was not done very well and
> that there was a lack of adequate testing?
> >
> > > As background, I implemented time handling for vector layers as a
> VOLUNTEER (completely unpaid). Would you have preferred I didn't do this,
> and we had no time support for vectors at all?
> >
> > As to what I would have preferred, well, I've reverted back to 3.10 to
> use the old TimeManager plugin to avoid using the NTC. I would have
> preferred that the feature branch you used to implement the NTC had not
> gotten merged and the feature included in release versions until it had
> been checked to work with and not break basic features like the selection
> tool. So now I have to work under some Damoclean time limit for a couple of
> months until 3.16 replaces 3.10 and I will have no choice but to move to a
> version with the NTC and these bugs.
>
> Full disclosure: I don't even really consider these issues as bugs.
> Functionality gaps, yes, but it's important to keep in mind that the
> new temporal framework was designed primarily around visualisation of
> time based data, and that using it for data analysis has been a
> secondary goal which has been slowly chipped away at since the initial
> implementation. The selection tool was NEVER broken, it was just never
> upgraded to be time aware. There's plenty of features in  QGIS which
> don't completely work alongside each other, and these are not always
> bugs but often are feature requests.
>
> I'd rather shelve that part of the discussion now, because I suspect
> we'll just keep going around in circles here at increasing levels of
> emotion, and I don't think that's healthy. I realise there's hurt
> here, but I DO think you are a valuable member of this community and
> I've much appreciated your previous diligence with submitting quality
> tickets. I'd rather move forward and find a way that we can move past
> this.
>
> Which leads me to a question: what exactly do you think should have
> been done differently here? The way I see it:
>
> 1. The temporal framework was in discussion and consultation phases
> for years prior to implementation. There were public calls for
> comments on the related QEPs, and consultation with all relevant
> stakeholders, including representatives from Kartoza (WMS-T), Lutra
> (mesh temporal handling), and the Time Manager plugin maintainers.  It
> was a group effort which pulled in ALL the expertise and experience of
> the QGIS project community.
>
> 2. QGIS already has some of the most stringent and rigorous code
> review processes around, and the pull requests implementing the
> temporal framework were subject to all these processes and peer
> review. There's plenty of developers who would attest to how difficult
> it is to get code merged into QGIS, due to the very strict coding
> guidelines and processes which govern the codebase. I fail to see
> anyway we could realistically further tighten these code review
> requirements without completely strangling the development of QGIS.
>
> 3. If you're asking for the pace of feature development to be slowed
> then that problem was addressed years ago when the LTR releases were
> introduced. The QGIS website is quite clear in advertising the LTR as
> the officially recommended version for stability and for mission
> critical work, while the non-LTR releases are clearly branded as
> "bleeding edge". I'd make the case that in the situation you've
> described you should never have been using the non-LTR release for
> this work without doing your full diligence to determine that it
> worked correctly in your workflow. And guess what? Most of the bugs
> are now fixed in time for 3.16.4, when 3.16 officially becomes the
> next LTR release! Again, I can't see how the project can do anything
> differently here. In fact, the next LTR (3.16) will even be supported
> for a massive 2 year period to ensure even more stable releases are
> available for mission critical work.
>
> So I ask again, in the full spirit of reconciliation and moving
> forward: what concrete, practical changes do YOU think the project
> should make?
>
> Nyall
>
>
>
> >
> > On Sun, Jan 3, 2021 at 9:32 PM Nyall Dawson <nyall.dawson at gmail.com>
> wrote:
> >>
> >> On Sat, 2 Jan 2021 at 10:23, Cory Albrecht <maps at hanfastolfe.com>
> wrote:
> >> >
> >> > Can somebody help me under the basics of how things work inside QGIS
> starting from when it loads all the features for a layer through the steps,
> and then finally drawing them on the map canvas, specifically with respect
> to the new temporal controller (NTC)?
> >> >
> >> > The issues caused by the NTC have been very frustrating for me as I
> make mostly (historical) timeline maps and I relied heavily on the old
> TimeManager (OTM) plugin by Antia Graser and group. So many tasks are now
> much more laborious or difficult because so many tools are just not
> time-aware.
> >> >
> >> > Was it not possible to add the NTC in such a way that would have
> still let all the other features work as before with the filtered feature
> set before being made time-aware, rather than confusingly operating on the
> unfiltered set? Or perhaps it shouldn't have been turned on until the
> infrastructure was there for the tools to be time-aware right away?
> >> >
> >> > Because I've just submitted yet another bug about the NTC, this time
> for the selection tool, and it has me more than a little annoyed. As a
> result of this bug I now have an unknown number of duplicate objects in
> multiple layers across multiple databases/projects that I unknowingly
> pasted into them over the past several months since the NTC was added to
> QGIS.
> >> >
> >> > I feel that the NTC was both poorly thought out and badly implemented
> as all these bugs would indicate.
> >>
> >> I can empathise with your frustration, but this is a very complex
> discussion!
> >>
> >> As background, I implemented time handling for vector layers as a
> >> VOLUNTEER (completely unpaid). Would you have preferred I didn't do
> >> this, and we had no time support for vectors at all? Please be very
> >> wary of your language in future -- every piece of feedback worded like
> >> this directly equates to a developer losing interest in volunteering
> >> their time on QGIS, to the harm of all.
> >>
> >> I've fixed two of your issues here
> >> https://github.com/qgis/QGIS/pull/40834, as an UNPAID VOLUNTEER.
> >> Without funding I'm just not interested in fixing the snapping related
> >> issues. I don't personally have any need for these, and the QGIS
> >> snapping code isn't something I'm motivated in getting involved with
> >> at all. Perhaps there's another developer interested in looking at
> >> this, or perhaps a developer will take a look at this during the 3.18
> >> bug sprint. Or maybe someone will pay for this fix and financial gain
> >> will be the motivation.
> >>
> >> I like making the world a better place through volunteering my time on
> >> making a first-class desktop mapping application, but I'm far from
> >> being a slave to this, and my family, garden, and drum kit want my
> >> time too!
> >>
> >> Nyall
> >>
> >>
> >>
> >> > _______________________________________________
> >> > QGIS-Developer mailing list
> >> > QGIS-Developer at lists.osgeo.org
> >> > List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
> >> > Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osgeo.org/pipermail/qgis-developer/attachments/20210106/4510e904/attachment-0001.html>