VBIndex / py_vb_toolbox

Vogt-Bailey index toolbox in Python
GNU General Public License v3.0
12 stars 12 forks source link

Added functionality to include cortex metadata in the output file #29

Closed KeithGeorgeCiantar closed 3 years ago

KeithGeorgeCiantar commented 3 years ago

At the moment, the files created by the VB Index do not include the necessary metadata to define which hemisphere they contain, this causes minor problems in programs like wb_view.

To fix this, I have added a small change, which takes the cortex information from the surface file, and adds it to the metadata of the output file. This makes it easier to load the files into the wb_view, without needing to manually specify the hemisphere.

LucasCampos commented 3 years ago

Hey @KeithGeorgeCiantar,

This seems like a fantastic commit. I cannot test it until the evening, but looking at the code, it all seems very good.

My only issue is that you actually do two things in this commit. Both what the title proposes, as well as the fixing of several typos in unrelated files.

Once testing is done, I will cherry-pick changes, and make two distinct commits.

KeithGeorgeCiantar commented 3 years ago

Hey @LucasCampos, thanks for the quick reply. I didn't realise that the typos and indents should've been in a different commits, but it makes sense to have them as distinct changes. I look forward to your further comments.

claudebajada commented 3 years ago

Hi @LucasCampos, any update on this?

My only issue is that you actually do two things in this commit. Both what the title proposes, as well as the fixing of several typos in unrelated files.

Once testing is done, I will cherry-pick changes, and make two distinct commits.

LucasCampos commented 3 years ago

Sorry, I completely forgot to test it. @claudebajada, did you by any change managed to test it in a different computer, just to check that the code is somewhat portable?

If so, we can merge already. Otherwise, I will have to test in the afternoon

LucasCampos commented 3 years ago

Done. Thanks, @claudebajada and @KeithGeorgeCiantar!