[Qgis-developer] [processing] OGR imports full of "None" strings, lately

[Sandro Santilli]

On Tue, Oct 18, 2016 at 12:45:10PM +0200, Sandro Santilli wrote:
> On Tue, Oct 18, 2016 at 12:37:39PM +0200, Victor Olaya wrote:
> > I made a fix to the check that verifies if a string parameter is left
> > blank or not. Before, it just checked that it was None, so empty
> > strings where considered valid values. However, an empty string should
> > be considered a null one (mainly, to raise an exception if that is
> > used for a parameter that is not optional). Before that, an empty
> > string was accepted even if the parameter was mandatory.
> > 
> > looks like ogr algorithms use unicode(getParameterValue()), and they
> > should instead get the param value and check whether is None or not.
> > 
> > Actually, there is no need to cast it to str or unicode, the value is
> > already a string. So it looks like ogr algorithms have to be modified
> > to adapat to this.
> > 
> > A simpler solution is that now, when a null value is passed (None,
> > empty string, anything that evaluates to false...), it sets the value
> > of the parameter as None. It could set it to an empty string, so no
> > need to do any change. Not such a clean solution, but it will work.
> > 
> > Any thoughts on that?
> 
> It sounds like having getParamaterValue() always return a string
> (possibly empty) would reduce regression probabilities.
> 
> Or do you think it's important to distinguish between empty string
> and None ?

Note that I see more than Ogr algortihms using the unicode(getParam())
construct:

  git grep  'unicode(self.getParameterValue' python/plugins/

As long as unicode(None) returns u'None', all those calls are bound
to fail if getParametervalue can possibly return None.

I agree it would be cleaner to drop the unicode() wrapper while making
sure len(val) is avoided. Not sure about the testcase coverage that
would help with such a big change.

--strk;