Skip to content

Initial implementation of almanac discovery API#103

Merged
delucchi-cmu merged 13 commits intomainfrom
issue/46/almanac
Jun 21, 2023
Merged

Initial implementation of almanac discovery API#103
delucchi-cmu merged 13 commits intomainfrom
issue/46/almanac

Conversation

@delucchi-cmu
Copy link
Copy Markdown
Contributor

Change Description

Initial implementation of almanac discovery API

Solution Description

This also adds a "factory" for generating typed catalog info that can be used by the almanac to parse the relevant fields.

Code Quality

  • I have read the Contribution Guide
  • My code follows the code style of this project
  • My code builds (or compiles) cleanly without any errors or warnings
  • My code contains relevant comments and necessary documentation

@delucchi-cmu delucchi-cmu requested a review from smcguire-cmu June 8, 2023 18:45
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 8, 2023

Codecov Report

Merging #103 (7bf654a) into main (43f80a2) will increase coverage by 0.04%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #103      +/-   ##
==========================================
+ Coverage   99.78%   99.83%   +0.04%     
==========================================
  Files          34       41       +7     
  Lines         950     1181     +231     
==========================================
+ Hits          948     1179     +231     
  Misses          2        2              
Impacted Files Coverage Δ
src/hipscat/catalog/dataset/dataset.py 100.00% <ø> (ø)
...rc/hipscat/catalog/association_catalog/__init__.py 100.00% <100.00%> (ø)
...catalog/association_catalog/partition_join_info.py 100.00% <100.00%> (ø)
...rc/hipscat/catalog/dataset/catalog_info_factory.py 100.00% <100.00%> (ø)
src/hipscat/catalog/index/__init__.py 100.00% <100.00%> (ø)
src/hipscat/catalog/margin_cache/__init__.py 100.00% <100.00%> (ø)
src/hipscat/catalog/source_catalog/__init__.py 100.00% <100.00%> (ø)
src/hipscat/inspection/almanac.py 100.00% <100.00%> (ø)
src/hipscat/inspection/almanac_info.py 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Copy Markdown
Contributor

@smcguire-cmu smcguire-cmu left a comment

Choose a reason for hiding this comment

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

A few comments, mostly about clarifying some things. Sorry it took a while to go through, it took me a few reads to get how the Almanac and all the catalog infos are initialized, but I think it makes sense!

Comment thread src/hipscat/catalog/dataset/catalog_info_factory.py Outdated
Comment thread src/hipscat/catalog/dataset/catalog_info_factory.py Outdated
Comment thread src/hipscat/catalog/dataset/catalog_info_factory.py
Comment thread src/hipscat/inspection/almanac.py
Comment thread src/hipscat/inspection/almanac.py
Comment thread src/hipscat/inspection/almanac.py Outdated
Comment thread src/hipscat/inspection/almanac.py Outdated
Comment thread src/hipscat/inspection/almanac.py
Comment thread src/hipscat/inspection/almanac_catalog_info.py Outdated
Comment thread src/hipscat/inspection/almanac_catalog_info.py Outdated
Comment thread src/hipscat/inspection/almanac_catalog_info.py Outdated
Copy link
Copy Markdown
Contributor Author

@delucchi-cmu delucchi-cmu left a comment

Choose a reason for hiding this comment

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

Thanks for your thorough review and great suggestions!

Comment thread src/hipscat/catalog/dataset/catalog_info_factory.py Outdated
Comment thread src/hipscat/inspection/almanac.py
Comment thread src/hipscat/inspection/almanac.py Outdated
Comment thread src/hipscat/inspection/almanac.py
Comment thread src/hipscat/inspection/almanac.py
Comment thread src/hipscat/inspection/almanac.py Outdated
Copy link
Copy Markdown
Contributor

@maxwest-uw maxwest-uw left a comment

Choose a reason for hiding this comment

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

looks good!! just had one question

Comment thread src/.pylintrc
Comment thread src/hipscat/inspection/almanac.py Outdated
@delucchi-cmu delucchi-cmu requested a review from maxwest-uw June 21, 2023 16:24
@delucchi-cmu delucchi-cmu merged commit e33790f into main Jun 21, 2023
@delucchi-cmu delucchi-cmu deleted the issue/46/almanac branch June 30, 2023 16:03
@delucchi-cmu delucchi-cmu mentioned this pull request Nov 21, 2023
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.

3 participants