NREL / flasc

A rich floris-driven suite for SCADA analysis
https://nrel.github.io/flasc/
BSD 3-Clause "New" or "Revised" License
32 stars 18 forks source link

[BUG] poetry version solving fails with floris and flasc #200

Closed aclerc closed 1 week ago

aclerc commented 3 months ago

Is there an existing issue for this?

Current Behavior

I use poetry to define Python environments. I am making a new internal project called wakesteer-design and would like to use both FLORIS and FLASC. When I try adding one it works, but when I try adding flasc after I already added floris I get an error:

Because no versions of flasc match >2.0,<3.0
 and flasc (2.0) depends on coloredlogs (>=10.0,<11.0), flasc (>=2.0,<3.0) requires coloredlogs (>=10.0,<11.0).
And because floris (4.1.1) depends on coloredlogs (>=15.0,<16.0)
 and no versions of floris match >4.1.1,<5.0.0, flasc (>=2.0,<3.0) is incompatible with floris (>=4.1.1,<5.0.0).
So, because wakesteer-design depends on both floris (^4.1.1) and flasc (^2.0), version solving failed.

I don't think this is a major problem because I can just add flasc and use floris since floris is a dependency of flasc.

Expected Behavior

I can set up a new poetry environment and run poetry add floris and then poetry add flasc and it works.

Steps To Reproduce

  1. run poetry new test-package
  2. run poetry add floris
  3. run poetry add flasc

Environment

- OS: Windows 10

Anything else?

No response

misi9170 commented 3 months ago

It looks like these lines in the semantic versioning of the FLORIS and FLASC requirements, respectively, are what are causing the issue: https://github.com/NREL/floris/blob/main/setup.py#L30 https://github.com/NREL/flasc/blob/main/setup.py#L34

We can likely move the FLASC dependency up to coloredlogs~=15.0 to solve the issue

aclerc commented 3 months ago

Thanks for looking at this!

I am also trying to add pandas-stubs to my project but that also has a problem, this time because numpy is v1. If it's possible to open FLORIS and FLASC to numpy v2 that would help. I can raise a separate issue for that if you prefer.

Because no versions of pandas-stubs match >2.2.2.240805,<3.0.0.0
 and pandas-stubs (2.2.2.240805) depends on numpy (>=2.0.0), pandas-stubs (>=2.2.2.240805,<3.0.0.0) requires numpy (>=2.0.0).
And because flasc (2.0) depends on numpy (>=1.20,<2.0)
 and no versions of flasc match >2.0,<3.0, pandas-stubs (>=2.2.2.240805,<3.0.0.0) is incompatible with flasc (>=2.0,<3.0).
So, because wakesteer-design depends on both flasc (^2.0) and pandas-stubs (^2.2.2.240805), version solving failed.
paulf81 commented 3 months ago

@aclerc I opened a pull request to address the coloredlogs issue (#202), @misi9170 maybe we can do a patch release once this is in to include the fix in main?

On numpy v2 this is a little trickier, over on FLORIS (https://github.com/NREL/floris/pull/939) when I made a similar bump to coloredlogs, I went through and made the changes for numpy v2 as well, but there was some pushback because it's still relatively new. I think we'll want to move to v2, both FLORIS and FLASC, but maybe need to wait just a little bit for it to be adapted more widely in our dependencies. Still, I'm open to argument we should do this now. Maybe @misi9170 or @rafmudaf you have a feeling on this one?

paulf81 commented 3 months ago

Hi @aclerc , #202 should have resolved the poetry issue and then @misi9170 has pushed a patch release containing the fix (https://github.com/NREL/flasc/releases/tag/v2.0.1)

Regarding numpy v2, we've been discussing here and the consensus for now is to wait a little while for numpy v2 to be more widely adopted. Out of curiousity I was looking at the pandas github to see what approach they were taking. It seems like they were making sure the code was able to run with v2 (I had previously done this for FLORIS and remember it was not too difficult). But I think there requirement is still numpy < 2. Maybe that is a nice balance until more packages move over? @rafmudaf @aclerc @misi9170 does that sound right to you?

misi9170 commented 3 months ago

@paulf81 , are you suggesting that we switch from numpy~=1.20 to numpy<=2? I'm ok with that, if we confirm that everything runs with numpy v2

paulf81 commented 3 months ago

I think the two lines are basically the same, I was just saying that while I understood pandas was 2.0 ready, they requiring < 2, which is maybe the approach we take (~=1.20 translates to something like >=1.2 & <2)

misi9170 commented 3 months ago

Ah sorry, my mistake; I agree that it's nice to stick to supporting a single major version and that moving to supporting v2 seems a bit premature at this stage, so I'd prefer to stick with numpy 1.x

paulf81 commented 2 weeks ago

@aclerc wanted to check if good to close this one now?

aclerc commented 1 week ago

wo lines are basically the same, I was just saying that while I understood pandas was 2.0 ready, they requiring < 2, which is

yes I tried poetry add floris then poetry add flasc and it works :)