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

Updated README.md #59

Merged
merged 3 commits into from
Oct 12, 2024
Merged

Updated README.md #59

merged 3 commits into from
Oct 12, 2024

Conversation

jaidh01
Copy link
Contributor

@jaidh01 jaidh01 commented Oct 7, 2024

Description

This update revamps the README.md file for the FaceRec project to improve clarity and structure. The changes enhance user engagement and address issue #55 related to documentation quality.

Fixes #55

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Changes were verified by reviewing the rendered output of the updated README.md on GitHub. Tests included:

  • Checked formatting and links.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Copy link

senior-dev-bot bot commented Oct 7, 2024

Hi there! 👋 Thanks for opening a PR. 🎉 To get the most out of Senior Dev, please sign up in our Web App, connect your GitHub account, and add/join your organization Devasy Patel. After that, you will receive code reviews beginning on your next opened PR. 🚀

README.md Outdated
Comment on lines 168 to 179
## Function Flow

1. `create_new_faceEntry()`: This function receives a POST request with an image and metadata. It extracts the face from the image, calculates the embeddings, and stores the data in the database.

2. `Data()`: This function sent a GET request to `/data` endpoint of FastAPI app to get the list of Face Entries from MongoDB.

3. `update()` :This function is used to update the details of the face entry in the database.

4. `read()` : This function sent a GET request with specific Employeecode to read the information related to that particular Employeecode.

5. `delete()` : This function is used to delete the specific Employee Data.

Copy link
Owner

@Devasy23 Devasy23 Oct 7, 2024

Choose a reason for hiding this comment

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

This is required, maybe you can add a sequence diagram to picture it more efficiently and in an appealing way

Copy link
Collaborator

Choose a reason for hiding this comment

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

@jaidh01 do make a sequence diagram

README.md Outdated
Comment on lines 138 to 144
## Project Structure

- `requirements.txt`: Contains the Python dependencies for the project.
- `API/`: Contains code of FastAPI application.
- `FaceRec/`: Contain all files of HTML,CSS and flask application.
- `main.py`: Contains code of to start FastAPI and flask together.

Copy link
Owner

Choose a reason for hiding this comment

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

Keep this but as a retractable section, also can you please update the folder structure

README.md Outdated
Comment on lines 145 to 166
## Database Schema

1. Create new connection in MongoDB and Connect using given url
`URL: mongodb://localhost:27017/8000`

2. Create database using
Database name: `DatabaseName`
Collection name: `CollectionName`

3. Add data by importing json file:
From 'database.mongo' folder -> `{DatabaseName}.{CollectionName}.json`

The database contains a `faceEntries` collection with the following schema:

- `id`: A unique identifier for the face entry.
- `Employeecode`: A unique employee ID associated with the image.
- `Name`: The name of the person in the image.
- `gender`: The gender of the person.
- `Department`: The department of the person
- `time`: The time the face entry was created.
- `embeddings`: The embeddings of the face image.
- `Images`: Base64 encoded image file.
Copy link
Owner

Choose a reason for hiding this comment

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

This is also required but not in the readme.md but in the sphinx docs so can you add that as well

@Devasy23
Copy link
Owner

Devasy23 commented Oct 7, 2024

@jaidh01 the pre-commit hook messed with the requirement.txt maybe that's why the pipeline isn't passing.

DEBUG that issue and if you can revamp some changes to the sphinx doc as well then I'll assign this ticket, level2 tag

@Devasy23
Copy link
Owner

Devasy23 commented Oct 7, 2024

Everything else looks good to me, wbu @devansh-shah-11 ?!

@jaidh01
Copy link
Contributor Author

jaidh01 commented Oct 7, 2024

Hi @Devasy23, I have done all the changes as you suggested.
Please spare a time to review it.

@Devasy23
Copy link
Owner

Devasy23 commented Oct 8, 2024

You still need to add the sequence diagram

@jaidh01
Copy link
Contributor Author

jaidh01 commented Oct 8, 2024

You still need to add the sequence diagram

I have already added it.
Screenshot (62)

4. `read()`: Sends a GET request with a specific `Employeecode` to read the related information.
5. `delete()`: Deletes the specific employee data.

## Sequence Diagram
Copy link
Contributor Author

Choose a reason for hiding this comment

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

From here the sequence diagram starts.

Copy link
Owner

Choose a reason for hiding this comment

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

This is the plant uml for diagram u may need to compile and render it and then put it here

@Devasy23
Copy link
Owner

Devasy23 commented Oct 8, 2024

And are you looking forward to add this changes to the sphinx documentation also, that is in the .rst files from docs/

@jaidh01
Copy link
Contributor Author

jaidh01 commented Oct 8, 2024

@Devasy23, could you please guide me through updating the Sphinx documentation? I have no prior experience with this, but I am eager to learn.

@Devasy23
Copy link
Owner

Devasy23 commented Oct 9, 2024

Hi @jaidh01 ,

Thank you for updating the README.md. To ensure consistency across our documentation, I'd like those changes to be reflected in our Sphinx documentation as well.

Sphinx is a powerful tool for generating documentation, typically used for Python projects. It processes .rst (reStructuredText) files, but you can also include Markdown files like README.md with a few configurations. Here’s a quick overview of what you'll need to do:

  1. Install necessary extensions:

Sphinx doesn't support Markdown out of the box, but you can add support for it with extensions. Please install recommonmark and myst-parser, two popular extensions for parsing Markdown files:

pip install recommonmark myst-parser

  1. Update conf.py:

In the Sphinx configuration file (conf.py), you’ll need to enable these extensions. Add or update the following lines in the file:

extensions = ['recommonmark', 'myst_parser']

To automatically include the README.md, add this:

source_suffix = {
'.rst': 'restructuredtext',
'.md': 'markdown',
}

  1. Include README.md in your index.rst:

In the index.rst (or wherever appropriate in your documentation), include the README.md file like this:

.. include:: ../README.md
:parser: myst

This will ensure that the changes from the README.md are parsed and included in the Sphinx-generated documentation.

References:

Sphinx official documentation: https://www.sphinx-doc.org/

Guide on using Markdown with Sphinx: https://myst-parser.readthedocs.io/

recommonmark GitHub repo: https://github.com/readthedocs/recommonmark

Let me know if you need any further clarification!

@Devasy23
Copy link
Owner

Any update on this @jaidh01 ?

1 similar comment
@Devasy23
Copy link
Owner

Any update on this @jaidh01 ?

@jaidh01
Copy link
Contributor Author

jaidh01 commented Oct 11, 2024

Any update on this @jaidh01 ?

I will update the sphinx docs by today evening.

@jaidh01
Copy link
Contributor Author

jaidh01 commented Oct 11, 2024

Hi @Devasy23, I have updated the sphinx docs.
And in the code, there was only single line change in the index.rst file. Where I just add this line: - :parser: myst at the end.

So, should I commit it or leave it?

@Devasy23
Copy link
Owner

@jaidh01 please take pull from main branch to solve conflicts

@jaidh01 jaidh01 reopened this Oct 11, 2024
Copy link

Hi there! 👋 Thanks for opening a PR. It looks like you've already reached the 5 review limit on our Basic Plan for the week. If you still want a review, feel free to upgrade your subscription in the Web App and then reopen the PR

@jaidh01
Copy link
Contributor Author

jaidh01 commented Oct 11, 2024

@jaidh01 please take pull from main branch to solve conflicts

I have taken the pull from the main branch, and I think it should now resolve the conflicts.

@Devasy23
Copy link
Owner

@jaidh01 please take pull from main branch to solve conflicts

I have taken the pull from the main branch, and I think it should now resolve the conflicts.

Cool,

  • I saw you sphinx changes but there aren't any, the readthe docs isn;t still parsing README.md file.
    image

  • You haven't put seq diagram, just the plantuml code that LLM generated.
    image

@Devasy23 Devasy23 merged commit 6b984ef into Devasy23:main Oct 12, 2024
3 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adding Live Contributor Profile Photos to the README.md
3 participants