Skip to content
New issue

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

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

Already on GitHub? # to your account

cmec and tests directories are being included in python package #858

Closed
xylar opened this issue Jul 13, 2022 · 5 comments · Fixed by #881
Closed

cmec and tests directories are being included in python package #858

xylar opened this issue Jul 13, 2022 · 5 comments · Fixed by #881

Comments

@xylar
Copy link
Contributor

xylar commented Jul 13, 2022

Because of this call to find_packages() without any arguments, I think you are picking up the cmec and tests directories within the python package.

packages = find_packages()

Changing this call to:

packages = find_packages(include='src/python')

would likely avoid this, though I haven't tested it myself. (The lines immediately above this do nothing because packages is replaced on this line.)

Here is a screenshot from the latest tar file from conda-forge:
image

This is not a good situation because it is unclear if cmec is coming from its own conda-forge package (as it should be) or from this package (not desired).

lee1043 added a commit that referenced this issue Oct 19, 2022
#858. (2) Remove not-in-use files (old demo, png, vcs-related). (3) Clean up
@lee1043
Copy link
Contributor

lee1043 commented Oct 19, 2022

Hi @xylar, thank you for pointing this out. In PR #881, I am excluding cmec and tests directories from find_packages.

@acordonez I don't think there is any function that needs to be imported from under the cmec directory, could you please confirm on this?

@tomvothecoder I am excluding tests directory because I think it is yet in use. Do you think that is okay? I also wonder whether tests directory should be re-included when it will be used?

@acordonez
Copy link
Collaborator

@lee1043 nothing in the cmec folder needs to be included in the pcmdi_metrics package.

@lee1043 lee1043 linked a pull request Oct 19, 2022 that will close this issue
@xylar
Copy link
Contributor Author

xylar commented Oct 19, 2022

@lee1043, great, thank you!

@tomvothecoder
Copy link
Collaborator

@tomvothecoder I am excluding tests directory because I think it is yet in use. Do you think that is okay? I also wonder whether tests directory should be re-included when it will be used?

Hey Jiwoo, yes this should be okay. As far as I'm aware, it is standard Python practice to not include the tests directory in the package build. This directory is used to store tests that are ran locally or in CI/CD builds.

@lee1043
Copy link
Contributor

lee1043 commented Oct 19, 2022

@acordonez @tomvothecoder thank you for your comments!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants