Skip to content

Refactor to have references in separate sub-directory; closes #49#77

Merged
dr-rodriguez merged 12 commits intoastrodbtoolkit:mainfrom
dr-rodriguez:reference-directory
Aug 2, 2024
Merged

Refactor to have references in separate sub-directory; closes #49#77
dr-rodriguez merged 12 commits intoastrodbtoolkit:mainfrom
dr-rodriguez:reference-directory

Conversation

@dr-rodriguez
Copy link
Collaborator

@dr-rodriguez dr-rodriguez commented Jul 23, 2024

This changes the database saving/loading logic to add an optional reference_database variable (defaults to "reference"). When set, any table set as reference will get stored in that sub-directory. This is backwards compatible in the sense that when loading a database, it will also check the parent directory if it doesn't file reference tables in the sub-directory.

Closes #49

@dr-rodriguez dr-rodriguez marked this pull request as ready for review July 23, 2024 20:59
@dr-rodriguez dr-rodriguez self-assigned this Jul 23, 2024
@dr-rodriguez dr-rodriguez requested a review from kelle July 23, 2024 20:59
@dr-rodriguez dr-rodriguez changed the title First pass at having references in separate directory Refactor to have references in separate sub-directory Jul 23, 2024
@dr-rodriguez dr-rodriguez changed the title Refactor to have references in separate sub-directory Refactor to have references in separate sub-directory; closes #49 Jul 23, 2024
@kelle
Copy link
Collaborator

kelle commented Jul 25, 2024

This looks great! One question, do we really want reference/ to be a sub-directory of data/? This structure doesn't necessarily help with our navigation challenge.

SIMPLE-db/
├─ data/
│  ├─ 2mass source.json
│  ├─ asource.json
│  ├─ source1.json
│  ├─ reference_tables/
│  ├─ source2.json

Maybe we should we also have a sources sub-directory?

SIMPLE-db/
├─ data/
│  ├─ Sources/
│  │  ├─ 2mass source.json
│  │  ├─ source2.json
│  │  ├─ asource.json
│  │  ├─ source1.json
│  ├─ Reference_Tables/
│  │  ├─ Publications.json
│  │  ├─ Telescopes.json

Another option is to make the data/ and the reference/ directories both at the top level.

SIMPLE-db/
├─ data/
│  ├─ 2mass source.json
│  ├─ asource.json
│  ├─ source1.json
│  ├─ source2.json
├─ reference_tables/
│  ├─ Publications.json
│  ├─ Telescopes.json

@dr-rodriguez
Copy link
Collaborator Author

dr-rodriguez commented Jul 28, 2024

I do think they should be under data/ since both source data and references are data; having a separate top-level directory could lead to loosing part of the data.
Having a second sub-directory for the sources is something I thought about, but didn't try out. Seeing how relatively simple building this was I can probably do the same for the non-reference tables.

I can explore that over the next week. For ease of comparison I'll probably create a new branch off of this one, but we ideally want to merge both together.

So with that in place, we would have sources/ and references/ both under data/ which I thinkn is probably the cleanest setup. (We can iterate if those should be the best sub-directory names)

@dr-rodriguez dr-rodriguez requested a review from arjunsavel August 2, 2024 17:04
@dr-rodriguez
Copy link
Collaborator Author

This is now ready to go. It stores references under data/reference and sources under data/source. Tests are passing and I was able to create a PR for SIMPLE using this refactor at SIMPLE-AstroDB/SIMPLE-db#541

Once this is merged, I'll make a major astrodbkit2 release and re-run the SIMPLE tests.

Copy link
Collaborator

@kelle kelle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just added some suggested wording to the documentation. I've seen lots of folks struggle with paths and directory structure so I tried to make the structure more explicit throughout.

@dr-rodriguez dr-rodriguez merged commit 4043a64 into astrodbtoolkit:main Aug 2, 2024
@dr-rodriguez dr-rodriguez deleted the reference-directory branch August 2, 2024 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Move reference tables out of data/ directory

2 participants