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

Glob #392

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Glob #392

wants to merge 12 commits into from

Conversation

pjotrp
Copy link

@pjotrp pjotrp commented May 29, 2021

Globbing multiple documents.

  • Fixed test that allowed 2 schemas to be passed in
  • Multiple documents on command line
  • Solve RDF compression because of shared placeholder ID (PubSeq style) or emit warning
  • Add real filename globbing

Later we may need to add chunking when these documents take all RAM.

@codecov
Copy link

codecov bot commented May 29, 2021

Codecov Report

Merging #392 (b8d9433) into main (012c5a7) will decrease coverage by 9.38%.
The diff coverage is 60.00%.

❗ Current head b8d9433 differs from pull request most recent head 1869e96. Consider uploading reports for the commit 1869e96 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main     #392      +/-   ##
==========================================
- Coverage   79.29%   69.90%   -9.39%     
==========================================
  Files          18       16       -2     
  Lines        3260     2911     -349     
  Branches      879      766     -113     
==========================================
- Hits         2585     2035     -550     
- Misses        442      677     +235     
+ Partials      233      199      -34     
Impacted Files Coverage Δ
schema_salad/main.py 59.67% <60.00%> (-15.61%) ⬇️
schema_salad/makedoc.py 10.22% <0.00%> (-72.78%) ⬇️
schema_salad/jsonld_context.py 74.09% <0.00%> (-5.43%) ⬇️
schema_salad/utils.py 80.35% <0.00%> (-2.98%) ⬇️
schema_salad/avro/schema.py 61.14% <0.00%> (-1.60%) ⬇️
schema_salad/validate.py 70.50% <0.00%> (-0.62%) ⬇️
schema_salad/python_codegen.py 95.20% <0.00%> (-0.07%) ⬇️
schema_salad/__init__.py
schema_salad/schema.py
... and 3 more

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 012c5a7...1869e96. Read the comment docs.

@pjotrp pjotrp marked this pull request as ready for review May 30, 2021 10:33
@pjotrp
Copy link
Author

pjotrp commented May 30, 2021

Added multiple document support and globbing. Also added a PubSeq test. Ready for review!

Copy link
Member

@mr-c mr-c left a comment

Choose a reason for hiding this comment

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

Thank you @pjotrp !

Re: https://github.com/common-workflow-language/schema_salad/pull/392/checks?check_run_id=2708947782#step:7:16

Can you run make format and then check the output of make flake8?

Re: https://github.com/common-workflow-language/schema_salad/pull/392/checks?check_run_id=2708947654#step:7:25

Please run make diff_pydocstyle_report as shown above and fix the complaint :-)

Re: https://github.com/common-workflow-language/schema_salad/pull/392/checks?check_run_id=2708948393#step:6:1173

Looks like the MANIFEST.in needs updating..

Feel free to run tox as the CI tests stopped once the first error was reached.

@pjotrp
Copy link
Author

pjotrp commented May 31, 2021

Will fix. Are you OK with the test data hosted in test/data/pubseq?

@mr-c
Copy link
Member

mr-c commented Jun 3, 2021

Will fix. Are you OK with the test data hosted in test/data/pubseq?

I can't think of a reason not to be. What's the total size? We install our tests so users can verify post installation - (via pip, apt, whatever)

@mr-c
Copy link
Member

mr-c commented Jun 24, 2021

Ping :-)

@pjotrp
Copy link
Author

pjotrp commented Jun 24, 2021

On my todo list. Probably end of July because of holiday and other things.

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.

2 participants