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 test installation from TestPyPi workflow #499

Merged
merged 3 commits into from
Nov 11, 2024
Merged

Conversation

schuenke
Copy link
Collaborator

@schuenke schuenke commented Nov 5, 2024

I added a retry loop in the installation from TestPyPi step to avoid the workflow to fail because the deployed package from the previous step is not yet fully available for download / installation.

As far as I know, there is no status / availability check available.

Copy link
Contributor

github-actions bot commented Nov 5, 2024

Coverage

Coverage Report
FileStmtsMissCoverMissing
src/mrpro/algorithms/csm
   inati.py24196%44
   walsh.py16194%34
src/mrpro/algorithms/dcf
   dcf_voronoi.py53492%15, 48–49, 76
src/mrpro/algorithms/optimizers
   adam.py20195%69
src/mrpro/algorithms/reconstruction
   DirectReconstruction.py281643%51–71, 85
   IterativeSENSEReconstruction.py13192%76
   Reconstruction.py502256%42, 54–56, 80–87, 104–113
   RegularizedIterativeSENSEReconstruction.py411759%96–100, 114–139
src/mrpro/data
   AcqInfo.py128298%174, 212
   CsmData.py29390%15, 82–84
   DcfData.py45882%18, 66, 78–83
   IData.py67987%119, 125, 129, 159–167
   IHeader.py75791%75, 109, 127–131
   KHeader.py1651790%25, 127–131, 158, 210, 221, 228–229, 232, 239, 278–289
   KNoise.py311552%39–52, 56–61
   KTrajectory.py69593%178–182
   MoveDataMixin.py1371887%15, 113, 129, 143–145, 207, 305–307, 320, 399, 419–420, 422, 437–438, 440
   QData.py39782%42, 65–73
   Rotation.py6743695%100, 198, 335, 433, 477, 495, 581, 583, 592, 626, 628, 691, 768, 773, 776, 791, 808, 813, 889, 1077, 1082, 1085, 1109, 1113, 1240, 1242, 1250–1251, 1315, 1397, 1608, 1690, 1846, 1881, 1885, 1996
   SpatialDimension.py2302191%33, 103, 128, 135, 141, 261–263, 276–278, 312, 330, 343, 356, 369, 382, 391–392, 407, 416
   TrajectoryDescription.py14193%23
   acq_filters.py10190%47
src/mrpro/data/_kdata
   KData.py1051685%108–109, 118, 126, 180–181, 216, 221–222, 241–252
   KDataRemoveOsMixin.py29293%44, 46
   KDataSelectMixin.py21290%48, 64
   KDataSplitMixin.py49394%51, 81, 90
src/mrpro/data/traj_calculators
   KTrajectoryCalculator.py25292%23, 45
   KTrajectoryIsmrmrd.py13285%41, 50
   KTrajectoryPulseq.py29197%54
src/mrpro/operators
   CartesianSamplingOp.py50492%49–50, 90, 116
   ConstraintsOp.py60297%46, 48
   EndomorphOperator.py65297%228, 234
   FiniteDifferenceOp.py27293%40, 105
   FourierOp.py77199%131
   Functional.py71593%20–22, 117, 119
   GridSamplingOp.py136993%72–73, 82–83, 90–91, 94, 96, 98
   LinearOperator.py1711293%55, 91, 190, 220, 261, 270, 278, 287, 295, 320, 418, 423
   LinearOperatorMatrix.py1581690%82, 119, 152, 161, 166, 175–178, 191–194, 203, 215, 304, 331, 359
   MultiIdentityOp.py13285%43, 48
   Operator.py78297%25, 74
   ProximableFunctionalSeparableSum.py39392%50, 103, 110
   SliceProjectionOp.py173895%44, 61, 63, 69, 206, 227, 260, 300
   WaveletOp.py120596%152, 170, 205, 210, 233
   ZeroPadOp.py16194%30
src/mrpro/utils
   filters.py62297%44, 49
   modify_acq_info.py17194%12
   slice_profiles.py46687%20, 36, 113–116, 149
   sliding_window.py34197%34
   split_idx.py10280%43, 47
   summarize_tensorvalues.py11918%20–29
   typing.py181139%8–23
   zero_pad_or_crop.py31681%26, 30, 54, 57, 60, 63
TOTAL473835393% 

Tests Skipped Failures Errors Time
1953 0 💤 0 ❌ 0 🔥 1m 40s ⏱️

Copy link
Contributor

github-actions bot commented Nov 5, 2024

📚 Documentation

📁 Download as zip
🔍 View online

@lrlunin
Copy link
Collaborator

lrlunin commented Nov 5, 2024

In my opinion (which can be potentially wrong) I would rather put this block to the Publish to TestPyPI step. This way we would make the upload of the package "smart", in sense that upload will ensure and guarantee that the version is available. It would make more sense in case if цу would like to perform more things related to the package. Then we can just set a trigger to this step ready instead of monitoring the state of the package in each upcoming sub-routine.

@schuenke
Copy link
Collaborator Author

schuenke commented Nov 5, 2024

In my opinion (which can be potentially wrong) I would rather put this block to the Publish to TestPyPI step.

I think I would at least make it another step, no? Or am I missing that pypa/gh-action-pypi-publish already has this feature implemented?

This way we would make the upload of the package "smart", in sense that upload will ensure and guarantee that the version is available.

Assuming pypa/gh-action-pypi-publish doesn't support this: what would be your idea to verify the availability? Directly checking in the projects metadata (https://test.pypi.org/pypi/mrpro/json) if the $VERSION$SUFFIX is available?

It would make more sense in case if цу would like to perform more things related to the package. Then we can just set a trigger to this step ready instead of monitoring the state of the package in each upcoming sub-routine.

Currently, I don't think we will do more checks on the TestPyPi package, but ofc you never know 😄

@lrlunin
Copy link
Collaborator

lrlunin commented Nov 5, 2024

I think I would at least make it another step, no? Or am I missing that pypa/gh-action-pypi-publish already has this feature implemented?

Oups, I mean an additonal step in the same group. You're correct.

Assuming pypa/gh-action-pypi-publish doesn't support this: what would be your idea to verify the availability? Directly checking in the projects metadata (https://test.pypi.org/pypi/mrpro/json) if the $VERSION$SUFFIX is available?

I briefly looked at the existing parameters and didn't find those. I'll raise an issue and ask if the the action would like to cover this thing. It sounds very reasonable for me.

@fzimmermann89 fzimmermann89 enabled auto-merge (squash) November 11, 2024 14:58
@fzimmermann89 fzimmermann89 merged commit a96b9c6 into main Nov 11, 2024
19 checks passed
@fzimmermann89 fzimmermann89 deleted the fix_testinstall branch November 11, 2024 15:01
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