[chirp_devel] [csv] Friendlier defaults for missingcToneFreq/rToneFreq columns in csv import - Fixes #1577

[chirp.cordless at xoxy.net]

Responses inline.

On Apr 24, 2014, at 8:59 PM, Tom Hayward - esarfl at gmail.com  wrote:
> On Thu, Apr 24, 2014 at 8:09 PM,  <chirp.cordless at xoxy.net> wrote:
>> Patch attached.
>> -dan
> 
> I haven't tested it yet, but I see a few issues with style.
> 
> - Don't mix tabs and spaces. We prefer spaces.

Sorry, I will try to pay more attention to that. As an old-school
C hacker, tabs are instinctive, but ever since some of my earlier
Python attempts didn't work because of indenting issues, I've
really tried to use only spaces. Guess I failed here.
Do you have any hints for making this visible?

> - 0 is a number. You can (and should) use True and False in Python to
> represent booleans. This is a style thing, but it becomes important
> when another developer does a test like "1 is True". This evaluates
> False because the "is" operator is testing identity; 1 and True are
> different things.

Okay. Understood.

> Here's a Python trick you may not be aware of:
> +        _file_has_rTone = 0
> +        _file_has_cTone = 0
> +        for header in headers:
> +    if header == "rToneFreq":
> +                _file_has_rTone = 1
> +    elif header == "cToneFreq":
> +                _file_has_cTone = 1
> 
> Is equivalent to:
> _file_has_rTone = "rToneFreq" in headers
> _file_has_cTone = "cToneFreq" in headers
> 
> Short and sweet. It will iterate the list twice instead of once, but I
> think that's acceptable for readability.

I didn't know the idiom, and certainly way more readable.
I'd actually felt a little guilty about iterating the header line
for every line in the file, but didn't see a way to factor that
without creating globals, which I didn't think would be wanted.
If doubling that again for readability is preferred, I have a
better calibration of the tradeoffs expected.

You didn't ask for a patch respin, shall I, or is this OK and I should
just take this as advice for future work?

Thanks and regards,

-dan