Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Shareable Kedro-viz backend implementation #1498
Shareable Kedro-viz backend implementation #1498
Changes from 67 commits
5af4d9f
283a4f5
96bb1d2
7de3f65
3a023f9
3c2b625
5da7410
4762c2f
4bc45e5
6081e85
d044bd1
b986c85
d0f7e65
703b64c
9ecb395
6b01350
8b91f7e
30e5f8a
fd75407
480bd51
e94f0ec
a6ecd7c
1232e92
737af74
9e24189
ef4617f
9e6d9a5
5b0a860
a6781ac
6d28f6b
16673a4
605c450
ad8d089
ae2992a
52a2a8a
a0e7052
0046755
ef093d2
77e9ad4
bf9f106
c300d74
2dab4c0
031c59a
a6e7ef0
7854c6a
d162a8f
1415f00
3533925
455eeec
948d420
aa2d5e4
b401f2f
4174d43
008624d
de88895
74132c6
44681d4
16a01e5
08551e3
b8720b4
13f4425
d69f7c5
f26a53e
a6ea86e
269ed26
72a7e18
4cea3e9
c3de82f
33f568d
890f92e
3809add
e912b92
d2de353
29a0fab
76bd6dd
635634b
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to be very
fsspec
specific, so I'd include that in the name to avoid confusion. Should this be "public" method? Is anything accessing this directly or will it only be called from within the viz backend?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
true, the reason I have kept this generic is incase we have more package compatibilties to test in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was a bit surprised it is using "wb" instead of "w". I expect the result of
encode_to_human_readable
should be text that is readable, but seems like it is returningbytes
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, it's currently returning bytes. I will have to create an issue to figure the best way to make this more clearer. But it would be out of scope for the current one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
main_loc
is a slightly vague variable name, I'd name this differently and also add some doc string to describe what this is.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps
main_path
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd change
loc
topath
here as well if it makes sense to you.