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

schema for Enhancement #30

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

schema for Enhancement #30

wants to merge 1 commit into from

Conversation

ErnestaP
Copy link
Contributor

No description provided.

@ErnestaP ErnestaP marked this pull request as draft May 24, 2022 12:58
@codecov-commenter
Copy link

codecov-commenter commented May 24, 2022

Codecov Report

Merging #30 (fb74dd1) into main (8bcb144) will increase coverage by 0.33%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #30      +/-   ##
==========================================
+ Coverage   96.67%   97.01%   +0.33%     
==========================================
  Files          48       54       +6     
  Lines        1414     1572     +158     
==========================================
+ Hits         1367     1525     +158     
  Misses         47       47              
Impacted Files Coverage Δ
dags/schemas/enhancement_schema.py 100.00% <100.00%> (ø)
dags/schemas/generic_parser_schema.py 100.00% <100.00%> (ø)
dags/schemas/parser_schema.py 100.00% <100.00%> (ø)
tests/units/schemas/test_enhancement_schema.py 100.00% <100.00%> (ø)
tests/units/schemas/test_generic_parser_schema.py 100.00% <100.00%> (ø)
tests/units/schemas/test_parser_schema.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8bcb144...fb74dd1. Read the comment docs.

@ErnestaP ErnestaP force-pushed the schemas branch 2 times, most recently from 24efd47 to 17c77c4 Compare May 25, 2022 16:52
@ErnestaP ErnestaP marked this pull request as ready for review May 25, 2022 16:53
@Neodia
Copy link
Contributor

Neodia commented May 30, 2022

I think we should let this PR open until we add the validation. When colliding the schemas with the code we can check more easily if there are problems in the types. This PR should serve as documentation until then. WDYT @ErnestaP @drjova


from schemas.generic_parser_schema import GenericParserSchema


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here


from schemas.parser_schema import ParserSchema


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

@ErnestaP
Copy link
Contributor Author

I think we can merge it since this code is not used at all. Later, we can have a painful time while solving merge conflicts.
Talking about checking is the problems with types- tests are written for that. For now, I'm testing with the input, and output we have from the parser (this data are saved in files). Afterward, we need to change these files, according to what we will get from the parser in QA. Finally, the code will be forced to change as well. We don't need to check it field by field, it is enough to run tests and sees what error it returns, and change code accordingly.

* Added schemas for publisher (Springer) parser
* Added schemas for generic parser
* Added schemas for enhancer output
* Added tests
* Added note about schemas in README
* ref:  cern-sis/issues-scoap3#65
@drjova
Copy link
Contributor

drjova commented Jun 20, 2022

@ErnestaP thank you for the code, I believe we are in a stage of investigating if the current setup works well on QA hence I will suggest to keep it open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants