Skip to content

Conversation

kklein
Copy link
Collaborator

@kklein kklein commented Aug 5, 2022

You can find a rendered version of the new example in the docs here:
https://datajudge--48.org.readthedocs.build/en/48/examples/example_twitch.html

@codecov
Copy link

codecov bot commented Aug 5, 2022

Codecov Report

Merging #48 (fd25e42) into main (1432d1b) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main      #48   +/-   ##
=======================================
  Coverage   93.90%   93.90%           
=======================================
  Files          15       15           
  Lines        1607     1607           
=======================================
  Hits         1509     1509           
  Misses         98       98           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@kklein kklein marked this pull request as ready for review August 11, 2022 10:03
@kklein kklein changed the title Draft example revolving around twitch data. Add example revolving around Twitch data to documentation Aug 11, 2022
@kklein kklein requested a review from ivergara August 11, 2022 10:32
Copy link
Collaborator

@ivergara ivergara left a comment

Choose a reason for hiding this comment

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

Excellent job!

Now let's write an actual specification, expressing our expectations against the data.
First, we need to make sure a connection to database can be established at test execution
time. How this is done exactly depends on how you set up your database. When using our
default setup with running `$ ./start_postgres.sh <https://github.com/Quantco/datajudge/blob/main/start_postgres.sh>`_,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably here would be good to mention it's a container-based setup.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point! I restructured this a bit to talk about the dockerized setup at the very beginning.

FAILED twitch_specification.py::test_func[NumericMean::public.twitch_v2 | public.twitch_v2] - Ass...
=================================== 4 failed, 4 passed in 1.80s ====================================

So we see that we might not want to trust version 2 of the data as is. What exactly do we
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
So we see that we might not want to trust version 2 of the data as is. What exactly do we
So we see that we might not want to blindly trust version 2 of the data as is, and that a new exploration might be in order. What exactly do we

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was unfamiliar with this usage of the expression 'in order' and took the liberty to rephrase slightly.

kklein and others added 4 commits August 12, 2022 10:48
Co-authored-by: Ignacio Vergara Kausel <ivergarakausel@gmail.com>
Co-authored-by: Ignacio Vergara Kausel <ivergarakausel@gmail.com>
@kklein
Copy link
Collaborator Author

kklein commented Aug 12, 2022

Thanks a lot for the blazingly fast review @ivergara :)

@kklein kklein merged commit 9593c2e into main Aug 12, 2022
@kklein kklein deleted the twitch_example branch August 12, 2022 09:29
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