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

Ensure BSK_RL is compatible with BSK installation #89

Closed
Mark2000 opened this issue Dec 1, 2023 · 5 comments · Fixed by #93
Closed

Ensure BSK_RL is compatible with BSK installation #89

Mark2000 opened this issue Dec 1, 2023 · 5 comments · Fixed by #93
Labels
enhancement New feature or request infrastructure Testing, documentation, and CI system

Comments

@Mark2000
Copy link
Contributor

Mark2000 commented Dec 1, 2023

Relevant to #14, since our released versions should probably match certain bsk releases

I think we should check on import that a minimum version of bsk_rl is installed (by version? by commit hash for develop?) and warn the user if it is not

@Mark2000 Mark2000 added enhancement New feature or request infrastructure Testing, documentation, and CI system labels Dec 1, 2023
@jbbennett91
Copy link

Any chance this is related to the value mismatch that occurred sometime last week?

Installing current versions of basilisk and bsk_rl, then testing with bsk_rl/examples/general_satellite_tasking/single_sat.py returns the following issue:

image

@Mark2000
Copy link
Contributor Author

Mark2000 commented Dec 5, 2023

Ah, good catch. I believe that's a result of #90, which adds a flag to the info sometimes that's not in the form (time, message). I'll open a PR tomorrow to address this.

A documentation system is currently being put together and that'll change how example scripts are formatted and handled. Currently, example scripts aren't being caught in the test suite; I'll make sure that they are tested when we make that change so that PRs that break common use patterns like this are avoided in the future.

@Mark2000
Copy link
Contributor Author

Mark2000 commented Dec 5, 2023

@LorenzzoQM, could you review #92 which refactors the previous PR?

@Mark2000
Copy link
Contributor Author

Mark2000 commented Dec 5, 2023

@jbbennett91 Should be resolved now with #92 . As an additional heads up, there's a PR that'll be switching us to a src-style package coming up, so you may need to uninstall and reinstall the package.

@Mark2000
Copy link
Contributor Author

Mark2000 commented Dec 5, 2023

Over in Basilisk, @patkenneally is looking into automated beta version number increasing (so that each commit increases bsk version to 2.2.1b1, 2.2.1b2, 2.2.1b3, etc). This will allow us to match our develop to specific bsk versions without needing to dig around for commit hashes.

Considering code like this in the bsk_rl/__init__.py file, only remaining question is where BSK_VERSION should be specified. Needs to be in the src dir to check at import, which I think is where this check should happen given the development cycle.

from pkg_resources import (
    DistributionNotFound,
    VersionConflict,
    get_distribution,
    require,
)

BSK_VERSION = "2.2.1b0"
try:
    require(f"Basilisk>={BSK_VERSION}")
except VersionConflict:
    warn(
        f"Basilisk version {BSK_VERSION} or higher is required for full functionality. "
        f"Currently installed: {get_distribution('Basilisk').version}"
    )
except DistributionNotFound as e:
    e._template = (
        "The '{self.req}' distribution was not found "
        "and is required by {self.requirers_str}. Install from "
        "http://hanspeterschaub.info/basilisk/."
    )
    raise e

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request infrastructure Testing, documentation, and CI system
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants