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

DM-41209: Make standalone, pip-installable lsst package #24

Merged
merged 10 commits into from
Nov 21, 2023

Conversation

taranu
Copy link
Collaborator

@taranu taranu commented Nov 9, 2023

This drops the setup.py for a pyproject.toml and adds eups support, following DM-41309.

PyPI publication may happen someday but not soon, given that one of the dependencies is a C++ header-only library.

Copy link
Member

@timj timj left a comment

Choose a reason for hiding this comment

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

I'm a bit unsure as to whether you expect pip install to work given there seem to be some dependencies that aren't on pypi.

authors = [
{name="Rubin Observatory Data Management", email="dm-admin@lists.lsst.org"},
]
classifiers = [
Copy link
Member

Choose a reason for hiding this comment

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

Add GPL license to the classifier list and maybe there is a Science/Research intended audience and topic Scientific/Engineering :: Astronomy (see astropy)?

pyproject.toml Outdated
"matplotlib",
"numpy",
"pydantic",
"pytest",
Copy link
Member

Choose a reason for hiding this comment

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

Test dependencies should not be in the core dependencies. You don't need to install pytest to use this package.

requires-python = ">=3.10.0"
dependencies = [
"astropy",
"gauss2d",
Copy link
Member

Choose a reason for hiding this comment

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

Is this on pypi?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No. I suppose it could be as it's a single repo and meson is pip-installable, although I'm not sure if the build configuration meets PyPI's requirements. For now, I configured the non-eups build to pip-install the bindings, so that they show up in pip list and are recognized as dependencies.

@@ -0,0 +1,3 @@
import pkgutil

__path__ = pkgutil.extend_path(__path__, __name__)
Copy link
Member

Choose a reason for hiding this comment

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

New line? And next file as well.

@@ -59,7 +58,7 @@ class AsinhStretchSigned(apvis.BaseStretch):
to logarithmic behavior, expressed as a fraction of the
normalized image. Must be in the range between 0 and 1.
Default is 0.1
"""
""" # noqa: W505
Copy link
Member

Choose a reason for hiding this comment

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

Is this because of the math? Can you put a backslash in after the / to fix that? Does it render okay in sphinx?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The math line is 10 characters too long, yes. Is a backslash supposed to split it? Anyway, most of this is copy-pasted from astropy and lightly modified. I haven't actually configured docs for the package yet so I'll do that on another ticket.

@taranu taranu merged commit e899dfe into main Nov 21, 2023
1 check passed
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.

2 participants