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

Lab test cleanup 2 #772

Merged
merged 25 commits into from
Jan 7, 2020
Merged

Lab test cleanup 2 #772

merged 25 commits into from
Jan 7, 2020

Conversation

fredkingham
Copy link
Contributor

So this means we use the same method for both the lab test summary api end point and the results view end point.

It also fixes the bug where observations of a form such as 22.08.2019 cause the lab tests to fail

@coveralls
Copy link

coveralls commented Dec 19, 2019

Coverage Status

Coverage increased (+0.3%) to 78.853% when pulling 4e04ea3 on lab-test-cleanup-2 into da4c541 on v0.18.

…ns we get a string that looks like -2 - 4 so regex is the best way to cut it
…time series we may aswell only send a single value
…t observations method to make the super loop simpler
…bservation is pending look for the next one with a real value and use that. If there is no other value, show 'pending'
elcid/api.py Outdated Show resolved Hide resolved
elcid/api.py Outdated Show resolved Hide resolved
elcid/api.py Outdated
observations = sorted(
observations, key=lambda x: x.observation_datetime
)
observations.reverse()
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't providing your own sorting function only to reverse it on the next line seem odd?
- ....to_timestamp() or similar ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So we were doing

        observations = sorted(
            observations, key=lambda x:- x.observation_datetime.toordinal()
        )

I think this is the new way is more readable. I think I prefer this way but I'm happy with either.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

much better idea... use the reverse=True argument...

@fredkingham fredkingham merged commit 8d82846 into v0.18 Jan 7, 2020
@fredkingham fredkingham deleted the lab-test-cleanup-2 branch January 7, 2020 19:20
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.

3 participants