Skip to content

Conversation

@Lestropie
Copy link
Member

Supersedes #2669.

Includes a portion of #2330.


So there's a few things to unpack here:

  • edited documentation 5ttgen hsvs -white_stem #2330 includes some additional LUTs, however they are interspersed with many other proposed changes. I have manually extracted specifically the new LUTs from that PR.

  • One issue with the contribution of such text files to the code base is that the number of line modifications can be disproportionate with respect to the volume of effort involved. This can unjustly undermine the volume of effort invested by others into modification of source code. One way that we have tried to mitigate this (admittedly with numerous failures to do so in the history prior to coming up with the idea) is by taking those changes that are primarily the result of automatic content generation / for which the line change count is disproportionately high with respect to effort invested, and attributing the corresponding commit to "MRtrixBot gitbot@mrtrix.org".
    I have used this in the context of this PR in two ways, which I hope is considered fair to all parties involved.

    • In edited documentation 5ttgen hsvs -white_stem #2330, I have attributed contribution of the novel ordered LUT to @nicdc for having invested the effort in creating a commit and PR. Whereas the original unmodified LUT, which originates from elsewhere and is explicitly attributed to another creator, I have attributed to MRtrixBot.
    • In Schaefer2018 7/17 Networks LUT files #2669, while there was undoubtedly effort involved in creation of these tables, they could have reasonably easily been generated from external LUT data using a relatively small amount of code; and whether this was done manually or using code, I do not think it is equivalent to >10,000 lines of code. So what I have done is attribute the two 200 parcel tables, being equivalent in size to the contribution from @nicdc, to @ppruc, as a total of ~500 lines; the other tables I have attributed to MRtrixBot.
  • There are two different versions of LUTs for the 200-node parcellation. The one from @nicdc in edited documentation 5ttgen hsvs -white_stem #2330 only contains the cortical parcels. Whereas all tables from Schaefer2018 7/17 Networks LUT files #2669 by @ppruc include the FreeSurfer sub-cortical GM parcel labels. Longer-term, I would prefer to have the ability to treat cortical parcellations and sub-cortical parcellations independently and aggregate them as one chooses; just as we do in this published work, and described in ENH labelconvert: Multiple inputs #2481. FOr here I'm content to simply add all files as-is; the software is in no way hard-coded to use these files, they are merely provided in case of utility.

  • While not "bug fixes", given that these changes are purely addition of text files that are not referenced in source code in any way, I'm content to push to master rather than deferring to dev.

@Lestropie Lestropie requested a review from a team November 24, 2024 00:22
@Lestropie Lestropie self-assigned this Nov 24, 2024
@Lestropie Lestropie force-pushed the labelconvert_schaefer branch 2 times, most recently from 6e83e1e to e99e168 Compare August 26, 2025 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants