Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[bug] Collection of (current) bugs with validation pipeline #58

Open
7 tasks
Brilator opened this issue Sep 22, 2023 · 15 comments
Open
7 tasks

[bug] Collection of (current) bugs with validation pipeline #58

Brilator opened this issue Sep 22, 2023 · 15 comments
Labels
Type: Bug Something isn't working

Comments

@Brilator
Copy link
Member

Brilator commented Sep 22, 2023

I'm currently working on a few ARCs that keep running into failing CQC pipelines due to divergent handling of ISA structure / naming / referencing between ARC commander (v0.5), ARCitect (v≤0.07, based on ARCCtrl).

Collecting here. Feel free to move to more relevant repo / discussion.

  • .arc (not being added by ARCitect) "message": "Actual entity is not present: ./.arc"
  • registering ISA files in investigation
    • ARCitect: Study File Name | studies/<studyName>/isa.study.xlsx
    • ARC commander: Study Assay File Name | <assayName>/isa.assay.xlsx
  • name of isa.assay.xlsx and isa.study.xlsx metadata sheets
    • ARCitect: isa_study; isa_assay
    • ARC commander: Assay, Study
  1. CQC currently based off latest ARC commander configuration and hence failing for all ARCitect ARCs
  2. ARC commander (obviously) fails on ARCitect-created ARCs (e.g. arc export, arc a list, arc a register)
@Brilator
Copy link
Member Author

@JonasLukasczyk @HLWeil

@Brilator
Copy link
Member Author

Also, we might need a converter / upgrader between ARC specifications

@HLWeil
Copy link
Member

HLWeil commented Sep 22, 2023

Hey,
I'm currently working on bringing all the functionality we implemented in https://github.com/nfdi4plants/ARCtrl to the ArcCommander. This should fix most of the problems as we built ARCtrl to be quite backwards compatible (E.g. metadata sheet names).
Still thanks a lot for the list! This will be immensely helpful for testing.

And as a heads up: Most important things are those that fail when written by ArcCommander and read by ARCtrl. The other way around wont be much of a problem in the future.

@Freymaurer
Copy link

I think this is a huge issue of arc-validate. As it does not support deprecated and modern arcs created by ARCtrl/ARCitect. @kMutagene @omaus validation should absolutly be based on ARCtrl, as we have extensive deprecation support there.

@kMutagene
Copy link
Member

kMutagene commented Sep 25, 2023

validation should absolutly be based on ARCtrl

absolutely not, because that would prevent partial validation of anything that cannot be parsed by ARCtrl. The whole point of this library is validation being independent of any implementation of the ARC data model.

This library is on the brink of a 2.0 release that also supports validating any spec.

See diff between main and V2

@omaus
Copy link
Collaborator

omaus commented Sep 25, 2023

I think this is a huge issue of arc-validate. As it does not support deprecated and modern arcs created by ARCtrl/ARCitect. @kMutagene @omaus validation should absolutly be based on ARCtrl, as we have extensive deprecation support there.

Validation library makes no sense at all if it is based on ARCtrl. Vice versa, you would need to restructure everything in ARCtrl to validation needs. Everything must be of option type in this scenario because otherwise the parsing fails.

@Freymaurer
Copy link

What about string literals?

@kMutagene
Copy link
Member

I think we talked about something similar before. arc validate will do structural validation based on ontologies. We can and should create an ontology that contains valid names for metadata sheets and also tracks which are synonymous and deprecated.

@Brilator
Copy link
Member Author

What's the progress on this?
Sorry to be annoying, but it keeps being frustrating for users to upload an ARC and get two "failed" emails right away.

@kMutagene
Copy link
Member

it keeps being frustrating for users to upload an ARC and get two "failed" emails right away

Is it the validate arc pipeline step that fails for this specific issue though? IIRC that is a problem of CQC running in an empty repo, which most certainly fails the other pipeline steps because there is no investigation file in a blank repo. One way to tackle this could be providing an arc template repo

@Brilator
Copy link
Member Author

Brilator commented Oct 17, 2023

Yes it is the validate ARC pipeline.

  1. It fails because ARCitect does not create a folder .arc
Screenshot 2023-10-17 at 17 05 56
  1. It fails for study and assay folders created with ARCitect (v 0.0.10):
Screenshot 2023-10-17 at 17 04 56 Screenshot 2023-10-17 at 17 03 52

@HLWeil
Copy link
Member

HLWeil commented Oct 18, 2023

Regarding (1.):
https://github.com/nfdi4plants/ARC-specification/blob/main/ARC%20specification.md
The .arc folder is not part of the ISA-specification. It was initially meant as a config folder for the ArcCommander. I think the validation should not fail because of it being missing.

@kMutagene
Copy link
Member

The .arc folder is not part of the ISA-specification. It was initially meant as a config folder for the ArcCommander. I think the validation should not fail because of it being missing.

We can hotfix that from the current pipeline before finishing 2.0, but i would want to keep these hotfixes on deprecated software to a minimum.

It fails for study and assay folders created with ARCitect (v 0.0.10):

this looks like a harder thing to pin down. If i find it immediately, we can also do a hotfix for this. Can you link a repo where this is happening @Brilator ?

What's the progress on this?

Slower than expected due to several external factors. It is being worked on though.

@kMutagene
Copy link
Member

@ brillator i have pushed hotfixes for both these issues:

It fails because ARCitect does not create a folder .arc
It fails for study and assay folders created with ARCitect (v 0.0.10):

i have tested it in production with an arc created with ARCitect v0.0.10: https://git.nfdi4plants.org/test/another-test

please give it a spin.

@Brilator
Copy link
Member Author

Works like a charm.
Much appreciated!!

Screenshot 2023-10-23 at 17 04 41

@kMutagene kMutagene moved this to Backlog in ARCStack Jun 5, 2024
@omaus omaus added the Type: Bug Something isn't working label Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working
Projects
Status: Backlog
Development

No branches or pull requests

5 participants