search

yoshida-lab / XenonPy

XenonPy is a Python Software for Materials Informatics
http://xenonpy.readthedocs.io
BSD 3-Clause "New" or "Revised" License
133 stars 57 forks source link

[RFC] Consistency checking after removing error descriptors #113

Open yk-tsuboi opened 5 years ago

yk-tsuboi commented 5 years ago

In XenonPy/xenonpy/inverse/iqspr/estimator.py, there is a bug.

https://github.com/yoshida-lab/XenonPy/blob/fefc5cc111de81563ae2f118392b9e6721866885/xenonpy/inverse/iqspr/estimator.py#L72-L77

'desc' and 'y' are merged if length of them are different. In previous sample/iQSPR.ipynb, there are bug relating this problem in cell In[11]: (Already fixed).

I guess you should check length of smiles list and property DataFrame. Following is my opinion.

if len(smiles) != y.shape[0]:
    raise RuntimeError('len(smiles) != y.shape[0]')

Thank you. And I apologize for my poor English.

TsumiNa commented 5 years ago

@stewu5 How do you think about this? Should we have to add a consistency checking after removing error descriptors.

stewu5 commented 3 years ago

At the beginning, we are expecting the user to do some basic check on their data set, so the proposed check was not added. At this point, we are considering to make XenonPy a more user friendly environment to work with. Starting from v0.5, we may slowly increase the amount of automatic checks after some new functionalities are fully in place. We will keep this issue open as a reminder. Thank you for your comment, and sorry for the super belated response.