-
Notifications
You must be signed in to change notification settings - Fork 0
initial nabat models/api script #122
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
base: main
Are you sure you want to change the base?
Conversation
…on to tasks, update NABat spectro generating process
* renames acoustic batch to recording based on conversations * add migrations
Add additional settings to the admin page
Updated queries: SoftwareId is a Kitware SoftwareId. Query VetQuery { query PresignedUrlQuery { |
Add in new query to get existing annotations and match them up with any thing we currently have:
|
bats_ai/core/admin/__init__.py
Outdated
'NABatRecordingAnnotation', | ||
'NABatCompressedSpectrogram', | ||
'NABatSpectrogram', | ||
'NABatRecording', |
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.
No action needed, just observing that these don't follow the convention of other classes here of ending with ...Admin
. Not sure if you want to keep that convention for these or move forward as-is.
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.
You're correct, I'll update it
client/src/api/api.ts
Outdated
export interface NATBatFiles { | ||
id: number, | ||
recording_time: string; | ||
recording_location: string | null; | ||
file_name: string | null; | ||
s3_verified: boolean | null; | ||
length_ms: number | null; | ||
size_bytes: number | null; | ||
survey_event: null; |
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.
Is there an argument to be made for moving this into NABatApi.ts
?
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.
Probably good idea.
client/src/api/NABatApi.ts
Outdated
getSpectrogram, | ||
getSpectrogramCompressed, |
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.
Do you think these functions should include NABat
in their names? Is there a reason they share names with functions in the non-NABat api.ts
?
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.
Good idea.
apiToken: { | ||
type: String, | ||
default: () => undefined, | ||
}, |
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.
For my own learning, why have the default be a function that returns undefined
instead of just using the empty string as the default?
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 should probably change it to String as PropType<string | undefined>;
@@ -85,7 +105,8 @@ export default defineComponent({ | |||
confidence, | |||
comments, | |||
updateAnnotation, | |||
deleteAnno | |||
deleteAnno, |
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.
Would it be a big change/touch a ton of files if we changed this to deleteAnnotation
?
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.
This is getting removed/disabled right now. But I'll change the name currently.
if not request.user.is_authenticated or not request.user.is_superuser: | ||
return JsonResponse({'error': 'Permission denied'}, status=403) | ||
existing_task = ProcessingTask.objects.filter( | ||
metadata__type='Updating Species', |
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.
Any value in using an enum for metadata.type
at this point? Maybe at least for this one we can use a variable for the string 'Updating Species'
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.
Implment this as an enum instead of.
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.
Can the commented out code here be removed?
|
||
@app.task(bind=True) | ||
def update_nabat_species(self): | ||
processing_task = ProcessingTask.objects.filter(celery_id=self.request.id) |
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.
Is this guaranteed to exist at this point or is that up to the code that starts this task?
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.
Make sure you check for the existence of the task or in the calling request doing something different.
Co-authored-by: Michael Nagler <[email protected]>
# @router.get("/filtered", response=List[ProcessingTaskSerializer]) | ||
# def filtered_tasks(request, status: Optional[str] = None): | ||
# if status and status not in ProcessingTask.Status.values: | ||
# return {"error": f"Invalid status value. Allowed values are {ProcessingTask.Status.values}."}, 400 | ||
|
||
# tasks = ProcessingTask.objects.filter(status=status) if status else ProcessingTask.objects.all() | ||
# return ProcessingTaskSerializer(tasks, many=True).data |
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.
Remove these lines.
# class ProcessingTaskSerializer(serializers.ModelSerializer): | ||
# class Meta: | ||
# model = ProcessingTask | ||
# fields = '__all__' |
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.
Remove these lines.
resolves #121
NOTE: Use the ./scripts/USGS/sampleURL.txt for testing, just inject your apiToken.
acoustic_batch_initialize
that does:ApiToken
andBatchId
it requests some basic information about the NaBat file for the batchIdAcousticBatch
using the metadata that we have/nabat/{recordingId}/?surveyEventId={surveyEventId}&apiToken={apiToken}
/nabat/spectrogram/{id}
route to display the spectrogramTODO:
Client Side
acoustic_batch/${id}/spectrogram
endpoint for getting the spectrogram dataacoustic_batch/${id}/spectrogram/compressed
endpoint for getting the compressed spectrogram dataBack-End
Clean up compressed spectrogram generation and prediction code. It's a bit duplicated in locations and could be improved.
20240404 TODO:
20250408 TODO:
20250411 TODO: