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 pandas failure #206

Merged
merged 2 commits into from
Dec 17, 2021
Merged

Fix pandas failure #206

merged 2 commits into from
Dec 17, 2021

Conversation

schallerdavid
Copy link
Collaborator

@schallerdavid schallerdavid commented Dec 17, 2021

Description

This PR investigates recent failures of the CI when using the PandasTools from RDKit (#204).

Todos

  • Identify problem

Status

  • Ready to go

@schallerdavid
Copy link
Collaborator Author

Hey @dominiquesydow and @t-kimber ,

I think I found the issue. It is about changes to PandasTools with the new RDKit release. However, I am having a hard time getting all CI runs green. It's mostly about a connection problem when requesting data online. This is completely random across the different Python versions and operating systems tested. Did you also observe such behavior recently?

@dominiquesydow
Copy link
Collaborator

Hi @schallerdavid, thank you for the fix!!

Could you please raise an issue that reminds us to un-pin rdkit again, once the problem is solved on their end?

Regarding the connection problems - I started to collect frequent trouble-makers here to address them in one go in a separate PR:
#199

For this PR, let's merge even if we have the usual connection CI problems.

Copy link
Collaborator

@dominiquesydow dominiquesydow left a comment

Choose a reason for hiding this comment

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

Good to go from my side, thank you!

@schallerdavid
Copy link
Collaborator Author

@dominiquesydow I can't merge without all tests passing. Do you have such power :) ?

I will give it a few more attempts.

@t-kimber
Copy link
Contributor

@dominiquesydow I can't merge without all tests passing. Do you have such power :) ?

I will give it a few more attempts.

Hi @schallerdavid , don't you have this option ?

  • Use your administrator privileges to merge this pull request.

@dominiquesydow
Copy link
Collaborator

I have the power :) I will merge now (the tests will rerun anyways after the merge, no need to wait).

@dominiquesydow dominiquesydow merged commit 126db3e into master Dec 17, 2021
@dominiquesydow dominiquesydow deleted the pandas_issue branch December 17, 2021 14:59
@schallerdavid
Copy link
Collaborator Author

Ohh, just saw you merged something, maybe it was for this reason.

@dominiquesydow
Copy link
Collaborator

@schallerdavid - I just checked, @t-kimber is right, you are an admin according to TOC's GH settings; so you can merge with admin rights in the future. You have the power, too :)

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

Successfully merging this pull request may close these issues.

3 participants