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 compat tests #584

Merged
merged 15 commits into from Dec 14, 2021
Merged

Fix compat tests #584

merged 15 commits into from Dec 14, 2021

Conversation

jan-janssen
Copy link
Member

No description provided.

@jan-janssen jan-janssen added the integration Start the integration tests with pyiron_atomistics/contrib for this PR label Dec 13, 2021
@jan-janssen jan-janssen linked an issue Dec 13, 2021 that may be closed by this pull request
Comment on lines 29 to 30
grep -v "pyiron_base" ../pyiron_atomistics/.ci_support/environment.yml > environment.yml
tail -n +4 .ci_support/environment.yml >> environment.yml
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if we update a dependency in base but not (yet) in atomistics? would this work or throw an error due to twice the same dependency with different versions?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To my understanding the last entry for the same package overwrites all previous ones

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not work ... here is an example Binder

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wrote a small python script to merge the environments https://github.com/pyiron/pyiron_base/blob/compatfix/.ci_support/condamerge.py

@jan-janssen jan-janssen marked this pull request as draft December 13, 2021 21:05
@jan-janssen jan-janssen marked this pull request as ready for review December 13, 2021 22:07
Copy link
Member

@niklassiemer niklassiemer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! We could only think about a common script/class to handle environments - we also have the script to update an environment.yml by providing package, old, and new version.

@@ -284,12 +284,12 @@ def _validate_viewer_configuration(config: Dict) -> None:
@staticmethod
def _validate_no_database_configuration(config: Dict) -> None:
if "disable_database" in config.keys() and config["disable_database"]:
if config["project_check_enabled"]:
if "project_check_enabled" in config.keys() and config["project_check_enabled"]:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am a bit surprised to see this in this PR, but the changes are ok to me. Did they pop up here or just sneak in?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They sneaked in

@jan-janssen jan-janssen merged commit 3e37ef5 into master Dec 14, 2021
@delete-merged-branch delete-merged-branch bot deleted the compatfix branch December 14, 2021 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integration Start the integration tests with pyiron_atomistics/contrib for this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compatibility workflow do not check dependency updates.
2 participants