94 convenience function to return qualitycontrol object

AllenNeuralDynamics / aind-data-access-api

Library to interface with AIND databases

MIT License

2 stars 0 forks source link

94 convenience function to return qualitycontrol object #95

Closed dbirman closed 1 week ago

dbirman commented 1 month ago

This PR refactors the utilities and adds to new ones, and then uses those to support a helper function that can return a valid QC object (if it exists, and deserializes properly)

dbirman commented 3 weeks ago

From teams discussion:

Either we don't validate it or we move this into a different repo to avoid pulling aind-data-schema as a dependency
Use aind-metadata-validator for the validation?

dbirman commented 3 weeks ago

Maybe we can organize it like: utils/

* s3.py

* docdb.py

* quality_control.py (or aind_data_schema.py, since helpers seems similar enough to utils)

Helen pointed out that there's a bunch of downstream dependencies on this stuff, so I think I should revert it and just put my utilities into their own file

jtyoung84 commented 3 weeks ago

Maybe we can organize it like: utils/
* s3.py

* docdb.py

* quality_control.py (or aind_data_schema.py, since helpers seems similar enough to utils)
Helen pointed out that there's a bunch of downstream dependencies on this stuff, so I think I should revert it and just put my utilities into their own file

I reckon it would introduce a breaking change. We can re-organize things later.

dbirman commented 3 weeks ago

Maybe util/ isn't the best name since it conflicts with utils.py. We could go back to helpers/?

jtyoung84 commented 3 weeks ago

Maybe util/ isn't the best name since it conflicts with utils.py. We could go back to helpers/?

helpers is probably okay

dbirman commented 3 weeks ago

Thanks Dan. Let me know if you need any help or clarifications. Are you still intending to rename the folder to utils/helpers?

Side note for next time- Contributor Guidelines contains branch naming conventions. It's the same across the core services. You can also link the issue in the PR message by writing "closes/fixes #{issue_num}"

I use Github to open branches directly linked to issues, this also auto-links the issue in the PR without needing to write the closes statement. If the contributor guidelines don't support that then we should update the guidelines because that workflow is much better than manually writing this stuff out.

I'll start a thread in the sci comp channel about this.