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

Fix PyPI package generation #122

Merged
merged 11 commits into from
Nov 1, 2021
Merged

Fix PyPI package generation #122

merged 11 commits into from
Nov 1, 2021

Conversation

gaurav
Copy link
Collaborator

@gaurav gaurav commented Sep 29, 2021

I found a bug in the PyPI package generation -- the generated .whl and .tar.gz files didn't actually contain the crdch_model.py program. Luckily, it was a pretty easy fix, and crdch-model v1.1.1 contains the files and seems to work with the Example Data Workflow.

I don't think the data files are being included, either; there's some example syntax in https://setuptools.pypa.io/en/latest/userguide/declarative_config.html that might help.

Closes #69.

Copy link
Member

@turbomam turbomam left a comment

Choose a reason for hiding this comment

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

@sujaypatil96 and I were playing around with packages = find: b ut this is better.

@gaurav
Copy link
Collaborator Author

gaurav commented Sep 30, 2021

We definitely need to add linkml-model as a prereq in the generated PyPI model.

@gaurav
Copy link
Collaborator Author

gaurav commented Oct 5, 2021

Okay, I think I've added linkml-runtime as a prereq.

@turbomam I haven't been able to figure out how to get setup.cfg to include crdch_model as the top-level object (i.e. you would still need from crdch_model import crdch_model to access the objects in it. However, I have figured out how to do that with Poetry (PR #127). It might be worth switching over to that if that feature is really important to you.

Another option would be to rename the file from crdch_model/crdch_model.py to crdch/model.py. That way, you'll need to run from crdch import model, which... works better? Maybe? I dunno.

I'd love to know your thoughts, and if we need more time, we can discuss this on the Thursday call.

@gaurav gaurav requested a review from turbomam October 5, 2021 02:52
@gaurav
Copy link
Collaborator Author

gaurav commented Oct 5, 2021

@turbomam Oh, I should have mentioned:

@turbomam
Copy link
Member

from crdch_model import crdch_model is fine with me, I just kept forgetting to write it that way. @sujaypatil96 is making a good case for Poetry, so I wouldn't object to that either. I'm looking at the 1.1.2 and 1.1.3 packages now.

Copy link
Member

@turbomam turbomam left a comment

Choose a reason for hiding this comment

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

I can access BodySite() after importing 1.1.2, 1.1.3 or 1.1.5 now.

@sujaypatil96
Copy link
Contributor

Looks good @gaurav.🚀 I tried the importing the module and the associated dataclasses like from crdch_model import crdch_model and everything works as expected.

Right, +1 for PR #127, I think this is all the more reason to start adopting poetry.

Copy link
Contributor

@sujaypatil96 sujaypatil96 left a comment

Choose a reason for hiding this comment

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

Actually, I think I figured out what's causing the problem:

package_dir=
    =crdch_model
packages=find:

[options.packages.find]
where=crdch_model

By adding the above snippet, we can import crdch_model in the same way as PR #127.

Copy link
Contributor

@sujaypatil96 sujaypatil96 left a comment

Choose a reason for hiding this comment

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

Commits from PR #138 have been added to this PR, so this is ready to be merged.

@sujaypatil96 sujaypatil96 merged commit 9fb6291 into main Nov 1, 2021
@sujaypatil96 sujaypatil96 deleted the fix-pypi-generation branch November 4, 2021 15:49
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.

Get the ccdhmodel python code into PyPI as a package
3 participants