astrogilda / tsbootstrap

tsbootstrap: generate bootstrapped time series samples in Python
https://tsbootstrap.readthedocs.io/en/latest/
MIT License
65 stars 5 forks source link

added use of uv instead of pip in CI.yml #134

Closed astrogilda closed 2 months ago

astrogilda commented 3 months ago

Pull Request Template

Description

closes #133

Type of change

Please delete options that are not relevant.

fkiraly commented 3 months ago

hm, that doesn't seem to have worked? It does not find dependencies of the package.

astrogilda commented 3 months ago

yep, trying to resolve this.

Best, Sankalp Gilda, Ph.D.

On Fri, Apr 12, 2024 at 10:30 AM Franz Király @.***> wrote:

hm, that doesn't seem to have worked? It does not find dependencies of the package.

— Reply to this email directly, view it on GitHub https://github.com/astrogilda/tsbootstrap/pull/134#issuecomment-2051872288, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFTOOHTZ2BAYLZQUOM53LU3Y47VWZAVCNFSM6AAAAABGDNXBHCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANJRHA3TEMRYHA . You are receiving this because you authored the thread.Message ID: @.***>

astrogilda commented 2 months ago

Error logs indicate scikit-learn-extras has some incompatibility with Python 3.12 on Windows --https://github.com/astrogilda/tsbootstrap/actions/runs/8745910408/job/24001870002?pr=134

Could someone help diagnose? is our only reasonably choice for now to skip testing for this particular combination of python and OS completely and warn the users not to install tsbootstrap here until we can verify that sklearn-extras has been appropriately updated?

astrogilda commented 2 months ago

I thought about ways to simplify the ci file. AFAIK uv requires a virtual environment, and each run statement essentially opens a new shell, necessitating activation of the created virtual environment.

On Thu, Apr 18, 2024, 8:04 PM Franz Király @.***> wrote:

@.**** requested changes on this pull request.

I'm quite concerned that the CI.yml gets much more complicated, and thus harder to maintain... any way around that?

Re the sphinx issu, I am not sure, but the obvious suspects are the lines changed in conf.py.

— Reply to this email directly, view it on GitHub https://github.com/astrogilda/tsbootstrap/pull/134#pullrequestreview-2010159653, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFTOOHR53ZMO3E7BXBK5W6LY6BNQHAVCNFSM6AAAAABGDNXBHCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAMJQGE2TSNRVGM . You are receiving this because you authored the thread.Message ID: @.***>

fkiraly commented 2 months ago

If this is the simplest it can be, then it feels a bit immature and risky to me. Perhaps later then, when there is a GHA or similar?

astrogilda commented 2 months ago

I disagree.. Given the small size of the CI file, I don't really see an issue here. Plus using uv instead of pip has non-trivially improved the runtimes already.

astrogilda commented 2 months ago

Cool, I can do that.

On Sat, Apr 20, 2024, 4:20 PM Franz Király @.***> wrote:

@.**** requested changes on this pull request.

Happy in-principle to try this out, could we add the "show dependencies" and "show branches" back though? I find that important for debugging.

— Reply to this email directly, view it on GitHub https://github.com/astrogilda/tsbootstrap/pull/134#pullrequestreview-2013143694, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFTOOHTFWAFP2GYUKWEN25TY6LEZTAVCNFSM6AAAAABGDNXBHCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAMJTGE2DGNRZGQ . You are receiving this because you authored the thread.Message ID: @.***>

astrogilda commented 2 months ago

image