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

Data.frame layers #83

Merged
merged 18 commits into from
Apr 26, 2022
Merged

Data.frame layers #83

merged 18 commits into from
Apr 26, 2022

Conversation

maelle
Copy link
Collaborator

@maelle maelle commented Mar 14, 2022

Fix #75
Fix #62
Fix #85
Fix #84

I've done my best to reduce the fixtures size.

@maelle maelle marked this pull request as ready for review March 14, 2022 14:59
@codecov-commenter
Copy link

codecov-commenter commented Mar 15, 2022

Codecov Report

Merging #83 (6ca0123) into master (5ada120) will increase coverage by 1.73%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #83      +/-   ##
==========================================
+ Coverage   85.71%   87.44%   +1.73%     
==========================================
  Files           5        5              
  Lines         231      239       +8     
==========================================
+ Hits          198      209      +11     
+ Misses         33       30       -3     
Impacted Files Coverage Δ
R/info.R 90.69% <100.00%> (+0.45%) ⬆️
R/layer_attributes.R 90.74% <100.00%> (+5.74%) ⬆️
R/layers.R 84.70% <100.00%> (+2.51%) ⬆️
R/client.R 84.61% <0.00%> (-1.93%) ⬇️

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 5ada120...6ca0123. Read the comment docs.

@maelle

This comment was marked as resolved.

@maelle

This comment was marked as resolved.

@maelle maelle mentioned this pull request Apr 5, 2022
@maelle maelle requested a review from salvafern April 7, 2022 12:04
@annakrystalli
Copy link
Collaborator

I've completed the merge but...

😭 😭 😭 Tests failing!! But they don't locally 🤷‍♀️

It looks like they are being triggered by guess_layer_format but I'm not sure why.
Could you try them @maelle ? Or do the error messages make any more sense to you?

@maelle
Copy link
Collaborator Author

maelle commented Apr 8, 2022

I'll fetch the PR!

@maelle
Copy link
Collaborator Author

maelle commented Apr 8, 2022

The tests pass locally for me too 😭 I'll re-launch the workflows.

@annakrystalli
Copy link
Collaborator

This is so sad. I'm not sure what's going on. The tests passed yesterday. Since, I merged master into this branch to resolve the merge conflicts. This means the vendor params is also now included. That I believe os the only real change. I did refresh all the fixtures after the merge (f0eff34) but they all worked mine locally.

@annakrystalli
Copy link
Collaborator

Digging into one error (https://github.com/EMODnet/EMODnetWFS/runs/5885060397?check_suite_focus=true#step:6:356) , of failing Error (test-filter.R:26:5): numeric filters work -- biology test:

Error: Error: GET https://geo.vliz.be/geoserver/Emodnetbio/wfs?service=WFS&version=2.0.0&typeName=Emodnetbio:mediseh_cor_abs_pnt&request=DescribeFeatureType (geo.vliz.be/geoserver/Emodnetbio/wfs-fb549b.json)

I can see a fixture for this response it is referring to (geo.vliz.be/geoserver/Emodnetbio/wfs-fb549b.json) but it's in folder tests/testthat/fixtures/biology-layers/geo.vliz.be/geoserver/Emodnetbio/ rather than ~/Desktop/EMODnetWFS/tests/testthat/fixtures/mediseh_posidonia_nodata/geo.vliz.be/geoserver/Emodnetbio

Merge branch 'df' of https://github.com/EMODnet/EMODnetWFS into df
@maelle maelle merged commit 9f46042 into master Apr 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants