SIMPLE-AstroDB / SIMPLE-db

BSD 3-Clause "New" or "Revised" License
11 stars 21 forks source link

refactor ingest_spectral_types function #539

Closed Exu-112 closed 1 month ago

Exu-112 commented 1 month ago

Short description: refactor ingest_spectral_types function to ingest one at a time and use the new ingest method

Link to relevant issue: Closes #481

kelle commented 1 month ago

Please take a look. If all is okie dokie, I'll add the 3863 modified JSON files with the new photometric column.

dr-rodriguez commented 1 month ago

Overall, this gets the job done. I think we can do better in terms of the function flow and styling. One of the things I see a lot of these days is giant functions that do more than one thing, which is bad design and hard to test. One way we could work towards catching that is implementing a linter to this project. I'm using ruff with some custom settings in one of my own projects and the McCabe complexity indicator is useful for this: https://docs.astral.sh/ruff/rules/complex-structure/

I recommend creating a few issues from the comments above and then we can approve and update the json files.

kelle commented 1 month ago

Excellent suggestions. I implemented most of them.