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

PyPI memary integration #29

Merged
merged 10 commits into from
May 9, 2024
Merged

PyPI memary integration #29

merged 10 commits into from
May 9, 2024

Conversation

kingjulio8238
Copy link
Owner

Requirement for PyPi Integration - setup.py

Requirement for PyPi Integration - setup.py
@kingjulio8238
Copy link
Owner Author

kingjulio8238 commented May 7, 2024

Additional edits to do before packaging:

Update the folder structure to have it as memary/memary folder - can we just rename 'src' as memary?

Requirement for PyPi - reviewers please suggest edits if needed
For PyPi integration - see setup.py
@kevinl424
Copy link
Collaborator

Uploaded to TestPyPI here: https://test.pypi.org/project/memary-test/.

TODO before merging:

  • switch to live PyPI, rename package to memary instead fo memary-test
  • link two README images in link above to cloud instead of local (see link above for which images)
  • test on live PyPI to make sure it works

Copy link
Collaborator

@seyeong-han seyeong-han left a comment

Choose a reason for hiding this comment

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

I hope you guys test the codes first and upload them.
Thanks.

pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
dist/memary_test-0.1.4-py3-none-any.whl Outdated Show resolved Hide resolved
@kevinl424
Copy link
Collaborator

kevinl424 commented May 9, 2024

After running python3 -m build from the root directory and starting a new virtual environment, running pip install dist/memary_test-0.1.3-py3-none-any.whl and changing the ChatAgent import to from memary.agent.chat_agent import ChatAgent (all the directory stuff would no longer be needed), the streamlit app works.

Following review

  • update the two images mentioned in above comment
  • push package

@kevinl424 kevinl424 requested a review from seyeong-han May 9, 2024 00:44
@kingjulio8238
Copy link
Owner Author

Note: pip install dist/memary-0.1.3-py3-none-any.whl works instead of memary_test

Copy link
Collaborator

@seyeong-han seyeong-han left a comment

Choose a reason for hiding this comment

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

I should have told you earlier, but I think it's better to rename src as memary.
There is no problem with operating codes, but the structure looks redundant.
It depends on your preference. I'm fine with this structure also.

pyproject.toml Outdated Show resolved Hide resolved
@kevinl424
Copy link
Collaborator

I should have told you earlier, but I think it's better to rename src as memary. There is no problem with operating codes, but the structure looks redundant. It depends on your preference. I'm fine with this structure also.

Was thinking under src may be better long term just in case we add some other module that goes along with memary, although not sure if that will happen. Prefer to keep it as it is but definitely open to changing it too.

@kevinl424 kevinl424 requested a review from seyeong-han May 9, 2024 22:07
seyeong-han
seyeong-han previously approved these changes May 9, 2024
Copy link
Collaborator

@seyeong-han seyeong-han left a comment

Choose a reason for hiding this comment

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

Thanks @kevinl424 @kingjulio8238 .
I tested PyPI build files and all good.
Also, I fixed the README image links to use the local diagrams images.
Now, it is good to publish PyPI package after merging this branch.

kevinl424
kevinl424 previously approved these changes May 9, 2024
@kevinl424 kevinl424 dismissed stale reviews from seyeong-han and themself via f504fae May 9, 2024 23:17
@kevinl424 kevinl424 requested review from kevinl424 and seyeong-han May 9, 2024 23:19
Copy link
Collaborator

@seyeong-han seyeong-han left a comment

Choose a reason for hiding this comment

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

LGTM

@seyeong-han seyeong-han merged commit d87ebd4 into main May 9, 2024
@kevinl424 kevinl424 deleted the kingjulio8238-pypi-integration branch May 13, 2024 03:55
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