[FieldTrip] Code style and Matlab versions

Sun Mar 24 20:58:35 CET 2019

Hi, Jan-Mathijs,

Thank you for the detailed reply. I'll take all this under advisement.

That style guide is nice and detailed. I'll spend some time with it.

 > In general, we shy away from fancy object-oriented stuff (with a few 
exceptions, but these pertain to functionality that typical users don’t 
need to be aware of), because our users are most often scientist with 
little computational background (nor interest).

Would you consider the use of packages (aka "namespaces")? Not for 
user-visible code, but for FieldTrip's internal use. These are the 
"+foo" folders that create namespace qualifications for the functions 
and classes contained in them. There seems to be a good amount of 
copy-and-pasted code in the various private/ folders in FieldTrip. 
Moving these to a +fieldtrip/+internal package could be a way to remove 
the code duplication and expose those functions for testing, but still 
make it clear they're undocumented stuff for FieldTrip's internal use, 
and, as with the private/ folders, caveat emptor if a user decides to 
call them directly. The "+internal" package is just a naming convention; 
no access is enforced by the Matlab interpreter. But I think it's a 
well-established convention in various programming languages at this point.

I can't find when packages were introduced in Matlab, but this article - 
https://blogs.mathworks.com/pick/2014/07/18/simple-namespaces-and-brilliant-function-handles/ 
- from 2014 says they've been around "for a number of years" and is 
"very mature" at that point. So it would be well supported by all the 
Matlab versions you're targeting.

 > The majority of the test functions in our test directory can only be 
run on our local compute cluster, since many of the functions require 
data that is only available locally.

Is this because the data is proprietary, or just because it's big and 
can't be stored in the repo?

Cheers,
Andrew

On 3/24/19 3:43 PM, Schoffelen, J.M. (Jan Mathijs) wrote:
> Dear Andrew,
>
> Thanks for reaching out. If I recall well, we have a page that 
> documents some coding guidelines, which by no means is exhaustive, on 
> http://www.fieldtriptoolbox.org/development/guideline/code/. I realise 
> that this is quite outdated, since it still mentions SVN as the 
> version control system used, which we abandoned quite a while ago. 
> Yet, I think that most of these guidelines still hold. Indeed the 
> recommendation would be to use 2 spaces for indents, but it could be 
> that this is somewhat inconsistent (although I’d expect that the 
> majority of the code is using this convention).
>
> We look favourable to any type of pull request (I just saw yours 
> posted), as long as they well-motivated and thought through. Sometimes 
> we choose to ignore lint warnings, though, for instance when coding 
> style optimizations are not sufficiently backward compatible, or when 
> readability of the code is severely compromised.
>
> In general, we try to support Matlab versions up to 5 years ago, so 
> these days that would mean that we would start to support 
> functionality that has been introduced from 2014 onwards. This being 
> said, sometimes we boost backward compatibility of functionality by 
> including drop-in replacements of core Matlab functions in the 
> compat/matlabltSOMENUMBER folders. For instance, the “string” datatype 
> functionality, introduced in matlab2016 has the useful “contains” 
> function, that we considered to be of sufficient value (cleaning up 
> many instances of ~isempty(strfind()) ), to be promoted to a drop-in 
> function. In general, we shy away from fancy object-oriented stuff 
> (with a few exceptions, but these pertain to functionality that 
> typical users don’t need to be aware of), because our users are most 
> often scientist with little computational background (nor interest).
>
> The majority of the test functions in our test directory can only be 
> run on our local compute cluster, since many of the functions require 
> data that is only available locally. The travis continuous integration 
> stuff that is deployed upon submitting a PR at the moment is not doing 
> anything meaningful (don’t tell anybody), and it will always pass. In 
> general, when adding new functionality, we typically try and create a 
> little test function that ideally (if possible) creates some toy data 
> from scratch, so that it does not depend on reading data from disk. In 
> principle, all test functions that don’t load in data should run 
> anywhere.
>
> I hope that these replies are by no means the end of it, and I am 
> looking forward to further interactions, either on this platform or 
> through github.
>
> Best wishes,
> Jan-Mathijs
>
>
> J.M.Schoffelen, MD PhD
> Associate PI, VIDI-fellow - PI, language in interaction
> Telephone: +31-24-3614793
> Physical location: room 00.028
> Donders Centre for Cognitive Neuroimaging, Nijmegen, The Netherlands
>
>
>> On 23 Mar 2019, at 19:42, Andrew Janke <floss at apjanke.net 
>> <mailto:floss at apjanke.net>> wrote:
>>
>> Hi, FieldTrip folks,
>>
>> I'm a software developer working with a student who's using FieldTrip
>> for their thesis, and would like to make a couple Pull Requests to the
>> FieldTrip project.
>>
>> What's the standard code style for FieldTrip? It looks like 2 spaces for
>> indents, but this isn't consistent between all the files.
>>
>> Would you want style-only Pull Requests? How about Pull Requests that
>> fix Matlab lint warnings ("code inspections")?
>>
>> I noticed in http://www.fieldtriptoolbox.org/faq/requirements/ 
>> <http://www.fieldtriptoolbox.org/faq/requirements/> that you
>> try to support older Matlab versions. What's the oldest Matlab version
>> you're currently targeting? I'm wondering what recently-added Matlab
>> features can be used yet, and which versions of Matlab I should run the
>> tests under.
>>
>> Speaking of which, how do you run the full FieldTrip test suite (the
>> stuff in the test/ directory)?
>>
>> Cheers,
>> Andrew Janke
>> floss at apjanke.net <mailto:floss at apjanke.net>
>> _______________________________________________
>> fieldtrip mailing list
>> https://mailman.science.ru.nl/mailman/listinfo/fieldtrip
>> https://doi.org/10.1371/journal.pcbi.1002202
>
>
>
>
>
>
> _______________________________________________
> fieldtrip mailing list
> https://mailman.science.ru.nl/mailman/listinfo/fieldtrip
> https://doi.org/10.1371/journal.pcbi.1002202

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.science.ru.nl/pipermail/fieldtrip/attachments/20190324/6235d9a1/attachment-0002.html>
-------------- next part --------------
_______________________________________________
fieldtrip mailing list
https://mailman.science.ru.nl/mailman/listinfo/fieldtrip
https://doi.org/10.1371/journal.pcbi.1002202