← 输入框|github.com|刷新
Skip to content

Sciquery - #15

Merged
mtaghiza merged 13 commits into
masterfrom
sciquery
Jun 25, 2024
Merged

mtaghiza merged 13 commits into
masterfrom
sciquery

Conversation

@mtaghiza

@mtaghiza mtaghiza commented Mar 2, 2022

Copy link
Copy Markdown
Contributor

Adding SciQuery functionality.

@glemson glemson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

General comment is that I think it would be good to introduce some classes, maybe one for SciQuery and one for RDBComputeDomain, against which one can call functions.
Could come with a bunch of initialization codes like SciQuery initializinng the RDBComputeDOmain-s and their dbcontexts the user has access to, the fileservice instance (generally there will be only one), setting default target volumes etc.

@glemson glemson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Manu, it would be good to turn this into a proper PR soon.
Leaving it as Draft so long means the final reviewers are getting a LOT of work to do.

@mtaghiza

mtaghiza commented Jun 6, 2022

Copy link
Copy Markdown
Contributor Author

@glemson Note that the amount of work is the same as before, as there are no other modules intended to be modified for this PR. Last commits are only minor updates on previous code in the same module.

@mtaghiza
mtaghiza requested review from amitschang, dmedv and shandy79 June 6, 2022 15:00
@mtaghiza
mtaghiza marked this pull request as ready for review June 6, 2022 15:01

@shandy79 shandy79 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I was able to successfully run the short and long demo notebooks from Getting Started/AAS2023/SciQuery in the Astronomy image on prod.

@glemson glemson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

go ahead.

@mtaghiza
mtaghiza merged commit 87a07af into master Jun 25, 2024
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