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

Check whether pyfunnel version should be fixed in setup.py #576

Open
AntoineGautier opened this issue Nov 4, 2024 · 4 comments
Open

Check whether pyfunnel version should be fixed in setup.py #576

AntoineGautier opened this issue Nov 4, 2024 · 4 comments
Assignees

Comments

@AntoineGautier
Copy link
Contributor

AntoineGautier commented Nov 4, 2024

Currently >= is used.

  • Rationale for >=: This is the standard practice to loosely specify the dependency version in setup.py so that new package installations do not make unnecessary changes to the user environment. (In contrast, strict version specification is used in requirements.txt to track the build & test environment used for the package development.) Using a loosely specified version also avoids releasing a new version of BuildingsPy whenever a new version of pyfunnel is released — unless of course the new version of pyfunnel breaks BuildingsPy (e.g., due to a change in function signatures or output formats).

  • Issue with >=: CI tests in Buildings and IBPSA do not specify a version of pyfunnel, but rather install it as a dependency of BuildingsPy. So which version of pyfunnel is actually used likely depends on the caching system of the CI tool, and one has to look at the install log to check it out.

  • Conclusion: I would keep the version of pyfunnel loosely specified in setup.py of BuildingsPy but I would strictly specify it in .travis.yml of Buildings and IBPSA.

@mwetter @FWuellhorst Do you agree?

@AntoineGautier AntoineGautier self-assigned this Nov 4, 2024
@mwetter
Copy link
Member

mwetter commented Nov 4, 2024

@AntoineGautier I agree with your suggestion.

@FWuellhorst
Copy link
Contributor

FWuellhorst commented Nov 5, 2024

Sounds fine for me. Another option is using extras_require. We could specify [CI] and tell travis and gitlab to pip install buildingspy[CI]:

setup(
    # Loose version in main requirements
    install_requires=[
        "pyfunnel>=0.3.0",
    ],
    # Strict version in extras
    extras_require={
        'CI': [
            "pyfunnel==0.3.0",
        ]
    }
)

@beutlich
Copy link

beutlich commented Nov 27, 2024

  • Conclusion: I would keep the version of pyfunnel loosely specified in setup.py of BuildingsPy but I would strictly specify it in .travis.yml of Buildings and IBPSA.

That is the recommened best practice. setup.py should use version ranges (to avoid version conflicts on dependencies), CI or Docker file should use pinned versions of all dependencies for transparency (SBOM) and reproducability.

@AntoineGautier
Copy link
Contributor Author

I will pin pyfunnel version in the CI configuration files of the IBPSA and Buildings libraries.

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

No branches or pull requests

4 participants