Closed sifaoufatai closed 3 months ago
This version doesn't exist anymore; I deleted it and implemented another one.
Le ven. 31 mai 2024 à 15:02, Sylvain Takerkart @.***> a écrit :
@.**** requested changes on this pull request.
Voilà un premier ensemble de commentaires sur la forme... Je vais regarder l'algorithmie plus tard... T'as déjà de quoi faire ;)
On bep32v01/agnostic_file_template.py https://github.com/INT-NIT/BEP032tools/pull/150#discussion_r1622319171:
this file seems to be a double of another file... only keep one! (cf my other comment on this other file)
On bep32v01/tests/TestAgnoticfile.py https://github.com/INT-NIT/BEP032tools/pull/150#discussion_r1622320076:
again, check the spelling of your filename: agnoStic
In bep32v01/tests/TestAgnoticfile.py https://github.com/INT-NIT/BEP032tools/pull/150#discussion_r1622322940:
@@ -0,0 +1,117 @@ +import unittest +import warnings +import os +import json +import csv +import yaml +from unittest.mock import patch, mock_open, MagicMock +from bep32v01.agnostic_file_template import data_description_json_template, \
you try to import from agnostic_file_template.py... is this why you want it to be a .py file???
that file should be a json file; and you should simply read it instead of importing from it...
On bep32v01/writing_agnoticfile.py https://github.com/INT-NIT/BEP032tools/pull/150#discussion_r1622323826:
the spelling of the filename should be agnoStic! please check everywhere!
In bep32v01/writing_agnoticfile.py https://github.com/INT-NIT/BEP032tools/pull/150#discussion_r1622325503:
+import json +import os +import warnings + +import yaml + +from bep32v01.agnostic_file_template import data_description_json_template, Citation_template, \
- Sample_template +from bep32v01.agnostic_file_template import participant_json_template
+# Assign the data description template +data_description_template = data_description_json_template
+data_participants_template = participant_json_template +# Path to the file containing the BIDS version +bidversion_path = 'ressources/schema/BIDS_VERSION'
the spelling should be bids, not bid; please check everywhere!
In bep32v01/writing_agnoticfile.py https://github.com/INT-NIT/BEP032tools/pull/150#discussion_r1622340380:
- writer = csv.writer(f, delimiter='\t')
- writer.writerow(primary_keys.keys())
- writer.writerow(primary_keys.values())
- elif path_to_save.endswith('.cff'):
- with open(path_to_save, 'w') as f:
- yaml.dump(primary_keys, f, default_flow_style=False)
- print(path_to_save)
- print(primary_keys)
- elif path_to_save.endswith('.txt'):
- with open(path_to_save, 'w') as f:
- for key, value in primary_keys.items():
- f.write(f"{key}: {value}\n")
- return primary_keys
+def complete_agnostic_file(output_dir, **kwargs):
rename this function by fill_agnostic_file
In bep32v01/writing_agnoticfile.py https://github.com/INT-NIT/BEP032tools/pull/150#discussion_r1622342267:
- primary_keys = {key: template[key]["Requirement Level"] for key in template.keys()}
- return primary_keys
+def get_agnostic_file_arguments(template):
- """
- Extracts arguments from the template to be used for creating an agnostic file.
- :param template: The template containing the data description.
- :return: A dictionary with argument keys and default values.
- """
- return {key: None for key in template.keys()}
+def write_agnostic_files(path_to_save, template, **kwargs):
you're writing only one file with this function, correct? if yes, put file on singular (file, not files, in the function name)
On bep32v01/template_agnotic_file/agonotic_files template.py https://github.com/INT-NIT/BEP032tools/pull/150#discussion_r1622318459:
and also, this file is called: template_agnotic_file/agonotic_files template.py
- you should correct the spelling so that you write agnoStic (in the directory name and in the filename)
- it seems to be a double of the other file at the root directory... only keep one! (cf my other comment on this other file)
— Reply to this email directly, view it on GitHub https://github.com/INT-NIT/BEP032tools/pull/150#pullrequestreview-2090662226, or unsubscribe https://github.com/notifications/unsubscribe-auth/A33HPUWSC5G22ZZQQSKNL3LZFBYHRAVCNFSM6AAAAABIMZ7YHSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAOJQGY3DEMRSGY . You are receiving this because you were mentioned.Message ID: @.***>
And i already made those modifications yet
Le ven. 31 mai 2024 à 15:23, sifaou fatai @.***> a écrit :
This version doesn't exist anymore; I deleted it and implemented another one.
Le ven. 31 mai 2024 à 15:02, Sylvain Takerkart @.***> a écrit :
@.**** requested changes on this pull request.
Voilà un premier ensemble de commentaires sur la forme... Je vais regarder l'algorithmie plus tard... T'as déjà de quoi faire ;)
On bep32v01/agnostic_file_template.py https://github.com/INT-NIT/BEP032tools/pull/150#discussion_r1622319171:
this file seems to be a double of another file... only keep one! (cf my other comment on this other file)
On bep32v01/tests/TestAgnoticfile.py https://github.com/INT-NIT/BEP032tools/pull/150#discussion_r1622320076:
again, check the spelling of your filename: agnoStic
In bep32v01/tests/TestAgnoticfile.py https://github.com/INT-NIT/BEP032tools/pull/150#discussion_r1622322940:
@@ -0,0 +1,117 @@ +import unittest +import warnings +import os +import json +import csv +import yaml +from unittest.mock import patch, mock_open, MagicMock +from bep32v01.agnostic_file_template import data_description_json_template, \
you try to import from agnostic_file_template.py... is this why you want it to be a .py file???
that file should be a json file; and you should simply read it instead of importing from it...
On bep32v01/writing_agnoticfile.py https://github.com/INT-NIT/BEP032tools/pull/150#discussion_r1622323826:
the spelling of the filename should be agnoStic! please check everywhere!
In bep32v01/writing_agnoticfile.py https://github.com/INT-NIT/BEP032tools/pull/150#discussion_r1622325503:
+import json +import os +import warnings + +import yaml + +from bep32v01.agnostic_file_template import data_description_json_template, Citation_template, \
- Sample_template +from bep32v01.agnostic_file_template import participant_json_template
+# Assign the data description template +data_description_template = data_description_json_template
+data_participants_template = participant_json_template +# Path to the file containing the BIDS version +bidversion_path = 'ressources/schema/BIDS_VERSION'
the spelling should be bids, not bid; please check everywhere!
In bep32v01/writing_agnoticfile.py https://github.com/INT-NIT/BEP032tools/pull/150#discussion_r1622340380:
- writer = csv.writer(f, delimiter='\t')
- writer.writerow(primary_keys.keys())
- writer.writerow(primary_keys.values())
- elif path_to_save.endswith('.cff'):
- with open(path_to_save, 'w') as f:
- yaml.dump(primary_keys, f, default_flow_style=False)
- print(path_to_save)
- print(primary_keys)
- elif path_to_save.endswith('.txt'):
- with open(path_to_save, 'w') as f:
- for key, value in primary_keys.items():
- f.write(f"{key}: {value}\n")
- return primary_keys
+def complete_agnostic_file(output_dir, **kwargs):
rename this function by fill_agnostic_file
In bep32v01/writing_agnoticfile.py https://github.com/INT-NIT/BEP032tools/pull/150#discussion_r1622342267:
- primary_keys = {key: template[key]["Requirement Level"] for key in template.keys()}
- return primary_keys
+def get_agnostic_file_arguments(template):
- """
- Extracts arguments from the template to be used for creating an agnostic file.
- :param template: The template containing the data description.
- :return: A dictionary with argument keys and default values.
- """
- return {key: None for key in template.keys()}
+def write_agnostic_files(path_to_save, template, **kwargs):
you're writing only one file with this function, correct? if yes, put file on singular (file, not files, in the function name)
On bep32v01/template_agnotic_file/agonotic_files template.py https://github.com/INT-NIT/BEP032tools/pull/150#discussion_r1622318459:
and also, this file is called: template_agnotic_file/agonotic_files template.py
- you should correct the spelling so that you write agnoStic (in the directory name and in the filename)
- it seems to be a double of the other file at the root directory... only keep one! (cf my other comment on this other file)
— Reply to this email directly, view it on GitHub https://github.com/INT-NIT/BEP032tools/pull/150#pullrequestreview-2090662226, or unsubscribe https://github.com/notifications/unsubscribe-auth/A33HPUWSC5G22ZZQQSKNL3LZFBYHRAVCNFSM6AAAAABIMZ7YHSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAOJQGY3DEMRSGY . You are receiving this because you were mentioned.Message ID: @.***>
And i already made those modifications yet
ah, ok, I don't know why, I was looking at the wrong commit... I see all your changes now!
let me test it...
Hello @sifaoufatai! Thanks for updating this PR.
There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers:
Comment last updated at 2024-05-31 13:35:16 UTC