ENH: Long-overdue update to Blender 2.8+

[marklescroart]

approximately working, needs further testing.

One big annoyance with blender 2.8 is that if you open a .blend file with blender 2.8+, you will no longer be able to open it with blender 2.7. Nuking backward compatibility seems bad, so what I have tried to do here is implement an automatic (and likely annoying) backup: every time you try to call a command that opens blender on an extant file, it creates a backup (_b2.7bkup.blend). This happens once for the file (presumably, the first time it is called with the updated code if the blender version is >= 2.8).

Putting a flag around these backups is potentially a good idea - something settable in the config file? e.g. make_legacy_backups=True, then once people decide they don't want a million _b2.7bkup files running around, and that they are committed to more recent blender, they can shut it off and delete those files?

This element is still a bit kludge-y, but it's aiming to prevent pain and suffering. Happy to take suggestions on how to better handle this. The compatibility with blender 2.8+ wasn't actually bad at all, just a few lines.

[TomDLT]

1. So a user can overwrite a blender file using pycortex. Is it always explicit, like calling a function cortex.blender.add_cutdata, or can a user overwrite a blender file without realizing it ?
2. Wouldn't it be better to always create a backup file, or to ask for confirmation when overwriting a file ?
3. Is there a way to check that a blender file is 2.7 or 2.8+ ? Then we could only trigger the backup if the file is a 2.7 file.
4. What are all the functions impacted by this ? Only the four functions in cortex.blender.__init__.py ? If the list is short, we could add a backup_27=True parameter to control this option. A config option is fine as well.

Minor comments:

• I would change the print into a UserWarning, which can be more easily silenced.
• I would change the suffix to "_b27bkup.blend", to avoid the confusing dot in the middle of the filename.

[marklescroart]

Thanks for feedback!

1. Hard to know what users will do. For example, it's entirely possible that a user might open up one of the cut-defining .blend files just by opening Blender and opening the file. I think I've caught each instance in which blender is opened by pycortex, because I've embedded the backup in the function pycortex uses to call blender.
2. asking for confirmation would probably be the best user experience; I've implemented that in an update that I'll push when I finish this. I'm posting the warning at file opening rather than at file saving, because (a) I'm not sure what auto-saving blender does, there's a chance that the file could be rendered un-usable once it's merely opened, or opened for a while, (b) I'm not sure how the input would work with blender's built-in terminal, and (c) it would be pretty annoying to open blender, do a lot of work, and only find out only when you went to save that your work might be incompatible with old version of blender.
3. Yes, implemented.
4. The list is indeed short. segment.py is kind-of-sort-of affected, in that it calls the blender functions, but I'm nearly sure I've got everything covered with the mods to those two files. Re: a config file flag governing backup, it seems less necessary now that backups are only being created for blender27 files (and user-confirmed). Once the file is saved w/ blender 2.8+, there isn't any value for backing up and there won't be a message. So the only ppl who would be annoyed by this would be ppl with large libraries of cut files that they are re-visiting for whatever reason. I think this is very few ppl.

Minor:
Now an input, so not a warning or a print statement
done

[TomDLT]

Great!

5. We might want to add some unit tests to avoid breaking this in the future, for example testing that _check_executable_blender_version and _check_file_blender_version work as expected.
6. I like the confirmation option, but I wonder if we should also have a parameter to skip it. Is it possible that a user would have to process many blender files in batch through pycortex ? If so, it might be annoying to have to confirm every time manually, and adding a parameter to skip the confirmation might be nice.
7. Finally, I wonder what Blender implements to deal with this issue. Is there a warning when opening a 2.7 file with 2.8+ Blender ? If not, it seems hard to prevent a user from messing with their files.

[marklescroart]

5. Unit tests are a great idea, always, but beyond scope for what I can manage RN. Specifically, writing a unit test that will require blender in potentially different versions and specific files to be present sounds like a thing that will delay the inclusion of this by potentially months if I have to find time to do it.
6. The only thing blender is used for is cuts - and the whole point of using it is to do them manually. I doubt there will be any batching of this. Not trying to be a jerk, but if anyone wants this, I would invite them to do it.
7. No, Blender does nothing re: warnings about backward compatibility breaking, which I think is idiotic (I found out the hard way). I forgive this because ver2.8+ is WAY better than past blender for many other reasons.

[mvdoc]

Could we just deprecate blender 2.7 and avoid having to track blender versions?

EDIT: my reasoning for this is: what will happen when Blender 2.9, 3.0 comes out? Will we have to change this logic once more? This doesn't sound very sustainable.

[TomDLT]

5.6.7. Fair enough. Given the fact that Blender does not warn for their backward incompatibility, I think there is no need to have a bulletproof solution in pycortex.

[marklescroart]

Current version checks work for 2.9 just fine, will work for all after. The version string checks return a tuple in the form of (major_version, minor_version), and it is checked with an inequality. Through dealing with this issue, I learned that you can do e.g. (1, 2) < (1, 4) and it works. Anyway, checks are for versions greater than or equal to, or less than, (2, 80). So code should be fine as it is for the foreseeable future (as much as anything is...)

[mvdoc]

suject -> subject

[marklescroart]

While we're at it, sure.

[mvdoc]

LGTM. Tests would be nice. But this is more of a helper function than a core pycortex function, so I guess we can be a bit relaxed about it...(last famous words?)

[marklescroart]

Tests never have existed for the surface-cutting functionality, as far as I know. We can add it as another issue on git to be addressed separately. It's complicated to test, in what would seem to be the correct way, because cutting a surface relies on blender (potentially multiple versions thereof), freesurfer, and an extant freesurfer subjects directory and data in there. There are also manual steps in this process, and catching all errors that might result from those is... pretty much impossible. So yes, I don't think there's a solution besides being a bit lax on best practices for this specific issue.