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

update rdflib-jsonld for py3-10 🐍⬆ #42

Merged
merged 5 commits into from
Jun 16, 2022

Conversation

shaikh-rashid
Copy link

This is a release of the candig/chord-metadata:v1.4.2 image built with python 3.10 and alpine 3.16. Please pull and test the image to verify that it works as expected. Testing can be done in candig-dev before being promoted to candig-prod.

@shaikh-rashid shaikh-rashid self-assigned this Jun 15, 2022
@kcranston
Copy link
Member

kcranston commented Jun 15, 2022

Looks like the tests are failing because the "rdflib-jsonld package has been integrated into rdflib as of rdflib==6.0.1. Please remove rdflib-jsonld from your project's dependencies."

The tests are running on python3.7, so it looks like dependency woes. Can we update the test python to 3.10 and see if we can get those passing?

@kcranston
Copy link
Member

The test suite uses nose, which is currently incompatible with python 3.10 xlcnd/isbnlib#95

@codecov-commenter
Copy link

codecov-commenter commented Jun 16, 2022

Codecov Report

Merging #42 (e6ba858) into develop (6d89420) will decrease coverage by 15.99%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##           develop      #42       +/-   ##
============================================
- Coverage    84.21%   68.22%   -16.00%     
============================================
  Files           95       56       -39     
  Lines         3941     1605     -2336     
  Branches       518      143      -375     
============================================
- Hits          3319     1095     -2224     
- Misses         412      505       +93     
+ Partials       210        5      -205     
Impacted Files Coverage Δ
chord_metadata_service/restapi/fhir_utils.py 8.02% <0.00%> (-77.60%) ⬇️
chord_metadata_service/restapi/argo_utils.py 13.79% <0.00%> (-70.69%) ⬇️
chord_metadata_service/patients/signals.py 73.33% <0.00%> (-26.67%) ⬇️
chord_metadata_service/phenopackets/serializers.py 67.93% <0.00%> (-20.62%) ⬇️
chord_metadata_service/resources/models.py 80.00% <0.00%> (-20.00%) ⬇️
chord_metadata_service/restapi/serializers.py 80.00% <0.00%> (-20.00%) ⬇️
chord_metadata_service/phenopackets/signals.py 57.44% <0.00%> (-15.53%) ⬇️
chord_metadata_service/restapi/validators.py 75.00% <0.00%> (-15.00%) ⬇️
chord_metadata_service/chord/models.py 83.69% <0.00%> (-14.04%) ⬇️
...hord_metadata_service/restapi/description_utils.py 88.23% <0.00%> (-11.77%) ⬇️
... and 45 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 6d89420...e6ba858. Read the comment docs.

Copy link
Member

@daisieh daisieh left a comment

Choose a reason for hiding this comment

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

This works fine on candig-dev.

@kcranston
Copy link
Member

This works fine on candig-dev.

Can you humour me with a little more detail? i.e. what tests did you run? I feel a little squeamish about merging this with failing CI without knowing more about the assertion that it is fine.

@shaikh-rashid shaikh-rashid merged commit adaf6e0 into develop Jun 16, 2022
@shaikh-rashid shaikh-rashid deleted the shaikh-rashid/py3-10_requirements branch June 16, 2022 20:16
@shaikh-rashid shaikh-rashid restored the shaikh-rashid/py3-10_requirements branch June 16, 2022 20:17
@daisieh daisieh deleted the shaikh-rashid/py3-10_requirements branch June 17, 2022 04:10
@daisieh
Copy link
Member

daisieh commented Jun 17, 2022

This works fine on candig-dev.

Can you humour me with a little more detail? i.e. what tests did you run? I feel a little squeamish about merging this with failing CI without knowing more about the assertion that it is fine.

I loaded this version on candig-dev and then checked the http://candig-dev.hpc4healthlocal:5080/katsu/api/datasets endpoint to see that it returned something. It was not a particularly rigorous test.

Of course, now that I've been (very reasonably) shamed about my lack of rigor in testing, I'm trying to ingest the synthetic dataset again on the server and noticed that I've missed an env var in docker-compose. I'll push that change to CanDIG/CanDIGv2#142.

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