-
Notifications
You must be signed in to change notification settings - Fork 910
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
Make import failures in kedro-datasets
clearer
#2943
Comments
Last time I saw this I was thinking the same, it turns out to be not trivial because of this. Not sure if things are still the same but worth mentioning. Lines 381 to 389 in 38dfe6f
|
Thanks @noklam - I see that an error saying "some of the dependencies have not been installed" is raised if the desired class is the list of Lines 392 to 396 in 38dfe6f
I see how we would not like to bubble all |
@astrojuanlu I think I come to similar conclusion, actually I am not sure why the issue disappear I thought I created it but I couldn't find it... |
I would push for this if it's possible. Definitely one of the most annoying thing and as a beginner you will hit this issue for sure (almost as annoying as the DataSet vs Dataset typo issue) |
kedro-datasets
kedro-datasets
do not give clear instructions of what went wrong
Updated the title of this issue and adding more evidence:
Indeed, I believe the same thing happened just this morning to @NeroOkwa. Maybe it would be better if the underlying In any case, this is something that should be changed on Kedro, so I'm moving the issue. |
Without reading the full context, are you aware of this is merged already? I think the error is surfaced now already (with kedro-datasets). I haven't fully tested it tho. |
This is what I see now:
doesn't tell me whether I made a typo, or what dependency I need to install. Am I missing something? This is with kedro-datasets 1.5.2 |
Will revisit that when we fixed the dataset installation issue. If this is not the case yet, I think we are in a good position to solve this finally. |
kedro-datasets
do not give clear instructions of what went wrongkedro-datasets
clearer
I want to have more clarification of this ticket. Is this a The original error mentioned in the description is something in
@astrojuanlu Would you be able to give me an fail cases so I can investigate this? |
As far as I understand, the logic to import datasets still lives in Kedro: Lines 389 to 395 in 93dc1a9
So in principle I think we can keep the ticket here, but up to you. Let me give a reproducer. |
(notice that soon this will happen with |
😅 from now on I may just repharse |
Today we found another example of this giving an unhelpful error message:
This was on top of kedro-org/kedro-plugins#402 (since that fix hasn't been released yet) and kedro-org/kedro-plugins#313. @noklam should I open a new issue? |
Do you have an example of that? Just checking how does this happen. |
@iamelijahko could you provide a series of steps to arrive to the error you saw initially? |
The screenshot depicts a sequence where the installation of a Kedro-related package with pandas support is attempted but fails due to missing system-level dependencies (HDF5). Following this, an attempt to start a Kedro IPython session also fails due to an issue with loading dataset definitions, which may or may not be related to the prior installation issue.
|
The installation issue was tracked in kedro-org/kedro-plugins#402 For clarity, the problem here is that the "is this a typo?" error message seems to hint that the dataset name is mistyped, when in fact the dependencies are missing. I'm just reopening. |
For the record, the problem that we used to have is fixed, this is failing for a separate reason. This happened because the installation fails, so NOTHING is installed, not just hdf5. It is prompting for You will see same error if you put
I tend to think this is a niche case and the priority is lower. I remove the estimate and move it back to |
Addressed in #3952 @ankatiyar ? |
I'm guessing so. Closing, but feel free to reopen if there's anything left. |
Description
Note
See #2943 (comment) for current status
Disambiguate the "module does not exist" error from the
ImportError
in messages like:This task will require investigation into how the error messages are swallowed, which will inform the proper implementation. Probably change:
kedro/kedro/io/core.py
Line 433 in 4194fbd
Context
This week I've been battling some puzzling documentation errors and after a while I noticed that, if the dependencies of a particular dataset are not present, the
ImportError
is swallowed silently. Examples:https://github.com/kedro-org/kedro-plugins/blob/b8881d113f8082ff03e0233db3ae4557a4c32547/kedro-datasets/kedro_datasets/biosequence/__init__.py#L7-L8
https://github.com/kedro-org/kedro-plugins/blob/b8881d113f8082ff03e0233db3ae4557a4c32547/kedro-datasets/kedro_datasets/networkx/__init__.py#L8-L15
This was done in https://github.com/quantumblacklabs/private-kedro/pull/575 to solve https://github.com/quantumblacklabs/private-kedro/issues/563 at the same time dependencies were moved to
extras_require
.I see how not suppressing these errors could be extremely annoying back then, because
kedro.io
used to re-export all the datasets in its__init__.py
:https://github.com/quantumblacklabs/private-kedro/blob/f7dd2478aec4de1b46afbaded9bce3c69bff6304/kedro/io/__init__.py#L29-L47
However, now our
__init__.py
is empty and datasets are meant to be imported separately:https://github.com/kedro-org/kedro-plugins/blob/b8881d113f8082ff03e0233db3ae4557a4c32547/kedro-datasets/kedro_datasets/__init__.py#L1-L3
So I think it would be much better if we did not silence those import errors.
More context
If one dependency is missing, the user would get an unhelpful "module X has no attribute Y" when trying to import a dataset rather than an actual error:
The text was updated successfully, but these errors were encountered: