-
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
CPHEAPM-27 Add ToxRefDB to Update Vocab Endpoint #1075
Conversation
# Conflicts: # frontend/animal/EndpointForm/App.js # frontend/animal/EndpointForm/TermSelector.js # frontend/animal/EndpointForm/components/VocabTermFields.js # frontend/animal/EndpointForm/constants.js # frontend/animal/VocabBrowser/App.js # frontend/animal/VocabBrowser/EhvBrowser/Table.js # hawc/apps/animal/forms.py # hawc/apps/assessment/models.py # hawc/apps/vocab/api.py # hawc/apps/vocab/constants.py # hawc/apps/vocab/migrations/0007_load_v2.py # hawc/apps/vocab/migrations/0008_alter_term_namespace.py # hawc/apps/vocab/models.py # hawc/apps/vocab/urls.py # hawc/apps/vocab/views.py # tests/hawc/apps/vocab/test_api.py # tests/hawc/apps/vocab/test_model.py # tests/hawc/apps/vocab/test_views.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.
Great job. I made a number of minor edits just rethinking how we do things in general, not specific to your updates. I think use of ToxRefDB data extraction in HAWC is now possible with this PR.
- a9a4a81 format (20 minutes ago)
- 6e0f54d use a lookup (20 minutes ago)
- 066b816 revise lookups (38 minutes ago)
- 363aedb remove debugging (63 minutes ago)
- dba741d text updates (64 minutes ago)
- 262144b add staticmethod props (65 minutes ago)
- 912527e fix api endpoint (65 minutes ago)
- 8ee535c minor edits from code review (2 hours ago)
- 4cae9d9 bump django version minimum (4 hours ago)
|
||
@property | ||
def display_url(self) -> str: | ||
return self.display_urls[self.value].lower() | ||
match self: |
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.
I switched to a match case instead of two methods for each; I suppose it may not be quite as speedy, but it should be plenty fast either way
Hi Andy, Sounds great, I appreciate you taking the time to review and make updates to the ToxRefDB PRs! I'll take a look through the commits and make note of any feedback / updates on how things are handled. Thanks, |
debug
from TermSelectorupdate_terms
API to work with EHV or ToxRefDB