Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Setup basic FastAPI project #10

Merged
merged 9 commits into from
Jul 12, 2022
Merged

feat: Setup basic FastAPI project #10

merged 9 commits into from
Jul 12, 2022

Conversation

aadarsh-ram
Copy link
Collaborator

@aadarsh-ram aadarsh-ram commented Jun 11, 2022

Fixes #6

What

  • Setup of basic FastAPI project, with a few endpoints
  • Dockerfile setup for the backend API
  • Docs containing Docker setup process
  • Creation of a JSON schema for a taxonomy
  • Creation of a JSON file for a test taxonomy

Part of

Copy link
Member

@alexgarel alexgarel left a comment

Choose a reason for hiding this comment

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

@aadarsh-ram just a comment to already structure the project.

api.py Outdated Show resolved Hide resolved
@aadarsh-ram aadarsh-ram marked this pull request as ready for review June 18, 2022 03:12
@teolemon teolemon requested a review from a team June 23, 2022 05:50
@aadarsh-ram
Copy link
Collaborator Author

@alexgarel Please review this PR, and let me know your comments.

Copy link
Member

@alexgarel alexgarel left a comment

Choose a reason for hiding this comment

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

@aadarsh-ram, good work.

I'm not ok with your shema.json and test.json, but let's merge first, for this is another problem.

For me we have to work with the neo4j database, maybe I should provide you with the sample data to import in neo4j which correspond to test.txt

@@ -0,0 +1,37 @@
# test taxonomy
Copy link
Member

Choose a reason for hiding this comment

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

Kudos for having wrote this test. This will be usefull to @BryanH01 also, I guess.

@@ -0,0 +1,50 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

I won't argue over the need of this schema.json, I really think we will do it all in the neo4j database. But let's get them in, for now we shall see later.

However there are mismatches with the spec.

For example:

  • "preceding_lines" is a list of strings.
  • no __comments__ block is mentionned
  • stopwords:IDX and synonyms:IDX are distinct nodes from __header__
  • etc.

But your schema has it's own logic per se… it's more than we must align :-)

Maybe we should better discuss the neo4j schema (in facts it's schema less) and have the equivalent of test.txt as an importable file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep, sure! We'll discuss the structure of JSON, and bring a unified schema.

@alexgarel alexgarel merged commit 1d1e789 into main Jul 12, 2022
@alexgarel alexgarel deleted the project-init branch July 12, 2022 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Set up basic FastAPI project
3 participants