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-41289: Make Felis fully pip installable #27

Merged
merged 2 commits into from Jan 10, 2024
Merged

Conversation

JeremyMcCormick
Copy link
Collaborator

@JeremyMcCormick JeremyMcCormick commented Dec 20, 2023

Configuration was updated so that felis will be pip installable using:

pip install felis

The DM developer documentation was used for guidance:

https://developer.lsst.io/stack/building-with-pip.html

Existing configuration was moved from setup.cfg to pyproject.toml where relevant.

A new job was added to build.yaml which publishes the package to PyPI.

A valid token needs to be added as a project secret called PYPI_UPLOADS for the publication to work.

@JeremyMcCormick JeremyMcCormick marked this pull request as draft December 20, 2023 23:17
Copy link

codecov bot commented Dec 20, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (72d3897) 98.47% compared to head (7014ae9) 92.68%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #27      +/-   ##
==========================================
- Coverage   98.47%   92.68%   -5.80%     
==========================================
  Files           7       18      +11     
  Lines         592     1915    +1323     
  Branches        0      379     +379     
==========================================
+ Hits          583     1775    +1192     
- Misses          9       86      +77     
- Partials        0       54      +54     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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.

This looks okay. Some minor comments.

Since this is a draft I will comment rather than approve. It looks like you need to look at the setup.cfg file and make sure that you have everything moved across (and then delete everything in it apart of the flake8 section which has to stay. The empty pytest config needs to move to pyproject.toml as well.

requirements.txt Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
@JeremyMcCormick JeremyMcCormick marked this pull request as ready for review January 4, 2024 22:38
@JeremyMcCormick
Copy link
Collaborator Author

This looks okay. Some minor comments.

Since this is a draft I will comment rather than approve. It looks like you need to look at the setup.cfg file and make sure that you have everything moved across (and then delete everything in it apart of the flake8 section which has to stay. The empty pytest config needs to move to pyproject.toml as well.

I moved all the configuration from setup.cfg to pyproject.toml.

The PR should be ready for review now.

@JeremyMcCormick JeremyMcCormick changed the title DM-41289: Make Felis fully pip installable (DRAFT) DM-41289: Make Felis fully pip installable Jan 4, 2024
@JeremyMcCormick JeremyMcCormick force-pushed the tickets/DM-41289 branch 10 times, most recently from c063831 to 0b2bcf5 Compare January 5, 2024 00:30
@JeremyMcCormick JeremyMcCormick force-pushed the tickets/DM-41289 branch 3 times, most recently from 691d4f7 to 5a360ae Compare January 5, 2024 20:17
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.

Looks good to me.

.github/workflows/build.yaml Outdated Show resolved Hide resolved
.github/workflows/build.yaml Show resolved Hide resolved
pyproject.toml Show resolved Hide resolved
Copy link

@gpdf gpdf 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 not really qualified to review pip-installability.

Tim's comments seem appropriate, so I look forward to a reply on those.

pyproject.toml Show resolved Hide resolved
.github/workflows/build.yaml Outdated Show resolved Hide resolved
@JeremyMcCormick JeremyMcCormick force-pushed the tickets/DM-41289 branch 3 times, most recently from ff3e140 to 76020e7 Compare January 10, 2024 22:37
Configuration was updated so that felis will be pip installable using:

    pip install felis

The DM developer documentation was used for guidance:

https://developer.lsst.io/stack/building-with-pip.html

Existing configuration was moved from setup.cfg to pyproject.toml where
relevant.
A new job was added to the build.yaml workflow for publishing to PyPI.
A few minor corrections and additions were made to the existing job for
building and running the tests.
@JeremyMcCormick
Copy link
Collaborator Author

I am merging without status checks passing because:

  • The 3.10 Python build is hanging, which should go away after the Github workflows are merged.

  • The codecov is reporting a failure because coverage percentage decreased, but this was due to a misconfiguration in running pytest.

@JeremyMcCormick JeremyMcCormick merged commit 617d492 into main Jan 10, 2024
9 of 10 checks passed
@timj timj deleted the tickets/DM-41289 branch January 11, 2024 00:01
@timj
Copy link
Member

timj commented Jan 11, 2024

I fixed the repo configuration to remove python 3.10 and prevent the required checks from being overridden.

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.

None yet

3 participants