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 #640: Jupyter integration conflicts with variable type converters from other integrations #641

Merged
merged 1 commit into from
Apr 15, 2024

Conversation

ark-1
Copy link
Contributor

@ark-1 ark-1 commented Apr 2, 2024

Fixes #640

@ark-1 ark-1 added the bug Something isn't working label Apr 2, 2024
@ark-1 ark-1 requested a review from Jolanrensen April 2, 2024 15:59
@ark-1 ark-1 self-assigned this Apr 2, 2024
@koperagen
Copy link
Collaborator

It was done on purpose, see the test. I think this should be fixed on kernel side, if possible. It's not obvious why when our function returns null no other converters are called

@ark-1
Copy link
Contributor Author

ark-1 commented Apr 2, 2024

I will discuss changing this behavior on the kernel side. Still, your case is already supported by a different API, so I suggest changing it nevertheless.

@koperagen
Copy link
Collaborator

Cool :) Then, run assemble and add generated sources to the commit please. No other questions from me

@Jolanrensen
Copy link
Collaborator

Yes, we need to make sure this #401 still works

@Jolanrensen
Copy link
Collaborator

All tests pass, also the ones introduced by #401, so should be good to go :)

@ark-1
Copy link
Contributor Author

ark-1 commented Apr 13, 2024

The problem with the proposed code that it didn't guarantee the ordering of checks and conversions.
I have rewritten the code to use full FieldHandler API, that has a separate accepts method.

@ark-1 ark-1 merged commit 195a305 into master Apr 15, 2024
3 checks passed
@Jolanrensen Jolanrensen deleted the ark/fix-640 branch April 15, 2024 11:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Jupyter integration conflicts with variable type converters from other integrations.
3 participants