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/issue 4850 #4851

Merged
merged 3 commits into from
Apr 8, 2021
Merged

Fix/issue 4850 #4851

merged 3 commits into from
Apr 8, 2021

Conversation

csadorf
Copy link
Contributor

@csadorf csadorf commented Apr 7, 2021

No description provided.

csadorf added 3 commits April 7, 2021 19:08
Utility function to analyze the package dependencies.
To avoid conflict for nest-asyncio required by both plumpy (dependency)
and  jupyter-client (sub-dependency).
To avoid conflict for coverage required by us with constrain <5 and
pytest-cov.
@csadorf csadorf added dependencies Pull requests that update a dependency file topic/dependencies labels Apr 7, 2021
@csadorf csadorf linked an issue Apr 7, 2021 that may be closed by this pull request
@csadorf csadorf added the topic/dependencies/constraint Issues related to dependency constraints that should be resolved. label Apr 7, 2021
@csadorf csadorf marked this pull request as ready for review April 7, 2021 17:35
@ramirezfranciscof
Copy link
Member

Cool, thanks for the fix @csadorf ! So were pytest-cov and jupyter-client the culprits?

One question though before aproving: the extra function you are adding, you used it to look for the problematic dependency and now want to add it for future use in similar cases, or is it part of the fix in some way I'm missing?

@csadorf
Copy link
Contributor Author

csadorf commented Apr 8, 2021

Cool, thanks for the fix @csadorf ! So were pytest-cov and jupyter-client the culprits?

Yes, I believe those were the culprits.

One question though before aproving: the extra function you are adding, you used it to look for the problematic dependency and now want to add it for future use in similar cases, or is it part of the fix in some way I'm missing?

I thought it would be useful to maintain the function for the next time we encounter such an issue. I could also move it to #4852, might be more appropriate, just let me know.

@ramirezfranciscof
Copy link
Member

Nah, that's fine. Also I don't know why the jenkins CI of the branch got stuck if the PR one worked, but I guess if the last one passed it should be ok.

Copy link
Member

@ramirezfranciscof ramirezfranciscof left a comment

Choose a reason for hiding this comment

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

Thanks, good to go!

@ramirezfranciscof
Copy link
Member

@csadorf should I merge or were you still waiting for @chrisjsewell review?

@chrisjsewell
Copy link
Member

I added unpinning to #4647

@csadorf
Copy link
Contributor Author

csadorf commented Apr 8, 2021

@csadorf should I merge or were you still waiting for @chrisjsewell review?

Please go ahead and merge. 👍

@ramirezfranciscof ramirezfranciscof merged commit 146e436 into develop Apr 8, 2021
@ramirezfranciscof ramirezfranciscof deleted the fix/issue-4850 branch April 8, 2021 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file topic/dependencies/constraint Issues related to dependency constraints that should be resolved. topic/dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to install aiida-core with all extras
3 participants