nfdi4plants / ARCCommander

Tool to manage your ARCs
MIT License
11 stars 9 forks source link

[BUG] Study Identifier written over by Assay Identifier in isa.investigation.xlsx #206

Closed andreaschrader closed 12 months ago

andreaschrader commented 12 months ago

Update(2023-09-11): used version number: v0.4.0

Describe the bug (Study) Assay Identifier is overwriting the Study Identifier upon arc update. There is no field for Study Assay Identifier

To Reproduce Steps to reproduce the behavior: arc init arc i create

fill in the Identifier e.g. SampleARC arc s init fill in the Identifier e.g. SampleStudy arc a init fill in the identifier e.g. SampleAssay arc update (or also in between) SampleARC is properly written in INVESTIGATION - Investigation Identifier SampleAssay is written in STUDY - Study Identifier. SampleStudie is not written in the isa.onvestigation file.

Expected behavior Find all three provided identifiers for the right keys in the isa.investigation file.

OS and framework information (please complete the following information):

andreaschrader commented 12 months ago

Just a quick additional note, "add" is still working nicely:

Previously I created the investigation and a study together with an assay in the command line (code below) which did not cause this problem (also arc a update works and does not casue an error) but users could also got his way with the current bug (above) of course.

arc init
arc i create -i SampleARC
arc assay add -s SampleStudy -a SampleAssay
arc a update

optional: "arc update" in between.

arc a init seems only to creates the directory without linking the respective assay to a study which requires to register it in addition while both is covered by arc add.

Quite confusing that one can initiate an assay without linking it to a study and when running arc update before registering this is eventually overwriting the study identifier.

HLWeil commented 12 months ago

I do not quite understand the problem, but in version 0.5.0 everything works as intended.

andreaschrader commented 12 months ago

I updated this issue with the version I used, sorry, wrote it in a previous issue but forgot it here.

Most users will work with the released version which is 0.4.0. Therefore, I go with this. When I inspected and searched through the issues, I did not find one relating to the exact problem I had observed maybe it was hidden in another bug reported. Therefore, I assumed that this particular behaviour is not yet known and/or addressed and reported it.

I installed the preview 2 temporaryly for you and also tested it in my setting and this behaviour did not occur. Thank you that you obviously also have already seen this problem and solved it in the preview of v0.5.0.

I added a warning in the knowledgebase in two PRs (https://github.com/nfdi4plants/nfdi4plants.knowledgebase/pull/226, https://github.com/nfdi4plants/nfdi4plants.knowledgebase/pull/225) for the sections on isa.study and isa.assay and also linked to the preview version which solves this issue.

Your fixes in the preview version of v0.5.0 obviously already fix this issue from the arcCommander's development perspective and the two PRs currently fix it from the user's perspective.

HLWeil commented 12 months ago

Alright, good to hear!

As 0.5.0 seems stable enough (only fixed bugs, non extra), I did a full release just now.