-
Notifications
You must be signed in to change notification settings - Fork 16
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
epiv2 CRUD api #820
epiv2 CRUD api #820
Conversation
* CRUD api endpoints for design, exposure measurements, chemicals, exposure_levels, outcomes, adjustment factors, data extraction * test cases for all of the above * a new ArrayedFlexibleChoiceField (plus tests), for FlexibleChoiceField-style behavior when the underlying field is wrapped in an ArrayField. * new BelongsToSameDesign mixin to validate that related objects belong to same overall design, e.g. when associating a chemical with an exposure level. * add environment variable (SKIP_BMDS_TESTS=1) to disable bmds testing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking great! I had some refactorings w/ the serializer mixin, but nothing major I dont think. Looks really clear
@shapiromatron went through feedback, just had outstanding questions on a few items so I left those unresolved. Let me know what you think |
Thanks - could you push up the latest version of code w/ updates? I only see a single commit |
* more consistent path naming * refactoring/renaming/cleanup of the SameDesignSerializer mixin
trying again; I am not getting any complaints when I run black locally, not sure if I have an out of date config? |
Did you run Line 72 in 3725ad3
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few more very minor changes/renames. Happy to chat if it's helpful to discuss.
* more consistent class names * rework the SameDesign serializer a bit more * etc.
# Conflicts: # hawc/apps/epiv2/api.py # hawc/apps/epiv2/urls.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fantastic work. I realized we forgot to update the hawc client to create these items. Perhaps it's something to chat about at our next meeting, since it sounds like ya'll will be using these APIs pretty soon...
assert len(set(converted)) == 1 and converted[0] == "job" | ||
|
||
for invalid_input in (["example", "bad input"], [1, "another bad input"]): | ||
try: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fyi, I changed this to use pytest.raises
for invalid_input in (["example", "bad input"], [1, "another bad input"]):
with pytest.raises(ValidationError):
numeric_choice.to_internal_value(invalid_input)
|
||
|
||
@pytest.mark.django_db | ||
def test_arrayed_flexible_choice_field(db_keys): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
converted to a class-based test to check each method separately
tests/hawc/apps/epiv2/test_api.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
amazing test suite; thank you for writing all of these
Enable create, read, update, and delete of all data in EpiV2 models via API:
In addition: