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

Jsvis prototype #12

Draft
wants to merge 32 commits into
base: main
Choose a base branch
from
Draft

Jsvis prototype #12

wants to merge 32 commits into from

Conversation

francoismg
Copy link
Contributor

@francoismg francoismg commented Apr 22, 2024

Prototype for js visualization

Still to do :

  • Add error bars processing for D3
  • More robust error bar processing for Bokeh
  • Code from data processor and wrappers needs to be improved
  • Some inconsistencies to fix and inheritance to lessen code duplication
  • Some clean up
  • make an npm package
  • custom columns on the error bar
  • delete column cross does not work
  • make bokeh a dependency
  • add arbitrary digits processing to arithmetic columns

@volodymyrss @dsavchenko

@francoismg francoismg marked this pull request as draft April 22, 2024 11:37
@francoismg francoismg mentioned this pull request Apr 22, 2024
@dsavchenko dsavchenko mentioned this pull request Apr 22, 2024
Copy link

github-actions bot commented Apr 23, 2024

PR Preview Action v1.4.8
🚀 Deployed preview to https://esg-epfl-apc.github.io/astrojsvis/pr-preview/pr-12/
on branch gh-pages at 2024-10-16 12:18 UTC

@volodymyrss
Copy link
Contributor

Looks interesting, shows in the preview! How do I load file in there?

@francoismg
Copy link
Contributor Author

Looks interesting, shows in the preview! How do I load file in there?

Right now it works like a galaxy plugin so file cannot be changed in the interface, if you want to try another file you have to add it to "test_files" and point to it in main.js

I could add a new box that is populated with a list of accessible files and let the user choose between them if you want.

It wasn't clear if the use case for a standalone version was a file or list of files or a specific endpoint is set in the code like with other js library and you instantiate the interface for a specific file or if it was up to the user to provide the files himself. Since there are probably way better tool to do that locally with local files I thought the main use would be that files are remote

@volodymyrss
Copy link
Contributor

Right now it works like a galaxy plugin so file cannot be changed in the interface, if you want to try another file you have to add it to "test_files" and point to it in main.js

It makes sense if it works like AladinLite, with an option to load local file or by URL.

I could add a new box that is populated with a list of accessible files and let the user choose between them if you want.

This would be useful. Else the demonstration does not actually demonstrate.

It wasn't clear if the use case for a standalone version was a file or list of files or a specific endpoint is set in the code like with other js library and you instantiate the interface for a specific file or if it was up to the user to provide the files himself.

We wanted it to be like AladinLite in the file load process. Presumably it's not actually difficult to select the source from upload and URL, right?

Since there are probably way better tool to do that locally with local files I thought the main use would be that files are remote

There is really not actually. UIs largely moved to the Web/Browser experience, but this sort of tool, similar to what Andrii was mentioning (called FV), was not re-implemented for Web. This is why we kind of have to do it from scratch.
But I think it starts to look very interesting!

@francoismg
Copy link
Contributor Author

Right now it works like a galaxy plugin so file cannot be changed in the interface, if you want to try another file you have to add it to "test_files" and point to it in main.js

It makes sense if it works like AladinLite, with an option to load local file or by URL.

I could add a new box that is populated with a list of accessible files and let the user choose between them if you want.

This would be useful. Else the demonstration does not actually demonstrate.

It wasn't clear if the use case for a standalone version was a file or list of files or a specific endpoint is set in the code like with other js library and you instantiate the interface for a specific file or if it was up to the user to provide the files himself.

We wanted it to be like AladinLite in the file load process. Presumably it's not actually difficult to select the source from upload and URL, right?

Since there are probably way better tool to do that locally with local files I thought the main use would be that files are remote

There is really not actually. UIs largely moved to the Web/Browser experience, but this sort of tool, similar to what Andrii was mentioning (called FV), was not re-implemented for Web. This is why we kind of have to do it from scratch. But I think it starts to look very interesting!

Ok no problem I will add a new box with an input that lets the user specify a file (url is the easiest right now but needs to come from a cors compliant server (or we can use an open cors proxy server), for local files uploaded directly I will just have to check fits-reader cause there was some specific things when dealing with local files) and once it's loaded it will be added to a list of available files that the user can select

@volodymyrss
Copy link
Contributor

Would be really cool if there was something for the demo today!

@francoismg
Copy link
Contributor Author

Would be really cool if there was something for the demo today!

I just finished error bars for d3 so not sure I will have the time but will try

@francoismg
Copy link
Contributor Author

For reference :

  • add the possibility for user to download and change current file (investigate possible way to do cross origin download)
  • synchronize hdu choice between header and data components
  • add extname to hdu name
  • add arithmetic and mathematical operation for column choice

@francoismg
Copy link
Contributor Author

  • Bugs with bokeh graphs have been fixed and error bars added to D3

  • I was able to plot the file from mmoda without changes to the code, only problem is the fact that timedel is from the header and changes regarding how this is handled are ongoing

  • For url file upload I looked at bypassing cors policies but I can't find a way to do that in client side js only without relying on some server config or code, using a proxy server or local proxy, or routing the requests through some backend. If you still have the code from that data challenge you talked about I could have a look at it maybe I have missed something

Regarding file upload, changes to the code and interface are still in progress and it's not testable right now but an usable minimal version should be ready during the week

Local file upload works with fits reader you just have to do things a bit differently that you would normally do but it works fine. However there were other things to take into consideration since users will be able to upload file directly, that csv support should be added, plus arithmetic column and multiple datasets support.

Ongoing development :

  • User will have to specify a file type before upload (fits and csv right now)
  • User will be able to upload any number of local files that will be displayed in an available files list
  • Files in the available list can be selected and parametrized to some extent (custom column names, header cards value change.....) + some general information about the files are displayed
  • User can move files from the available list to a "current" files list that will be used for generating visualization
  • User can remove files from the current list when necessary
  • Settings let the user choose between any column of any hdu or files that are in the current files list (every column is prefixed by a file id (and an hdu index when applicable)). That will let the user be able to plot things using data from any number of files together and do column arithmetic with columns from any files
  • Multiple datasets can be plotted on the same graph
  • Header and data component will still be synchronized together but will be able to show data from any files in the current list

@dsavchenko
Copy link
Contributor

@francoismg I made a small change so that the file that's in the repo is accessible in preview
But now there are two files in "available" and only the second one is really working. But I hope you can debug this easily

@francoismg
Copy link
Contributor Author

@francoismg I made a small change so that the file that's in the repo is accessible in preview But now there are two files in "available" and only the second one is really working. But I hope you can debug this easily

yes I have the same thing locally it was just some test I made, I will remove that so it's not confusing

@volodymyrss
Copy link
Contributor

@volodymyrss
Copy link
Contributor

How is it going, @francoismg ?

@francoismg
Copy link
Contributor Author

francoismg commented May 22, 2024

How is it going, @francoismg ?

Custom range is working for D3 still a few bugs but seems to work, I will push it so you can test that, just need to remove things that are not working. If you want to test it you need to select D3, x range is working fine but y range needs x range to be set to work (one of the bugs I'm working on).

Bokeh part should be good soon and hopefully the plugin part and the other unrelated bugs I have spotted could be ready for friday

@francoismg
Copy link
Contributor Author

The visualization package is now available as an npm package as "astrovisjs" version 0.9.0

Still need to test it locally and inside galaxy latest dev branch to be sure it's ready to open a pull request to galaxy repo and fix arithmetic columns issues

@francoismg
Copy link
Contributor Author

francoismg commented Sep 23, 2024

I have fixed most of the issues regarding importing bokeh and d3 as npm package however there's still an issue regarding bokeh when migrating from 3.3.* to 3.5.* -> Type error signal is undefined when trying to instantiate a graph

It seems like a known issue or at least related to :
bokeh/bokeh#13864
bokeh/bokeh#13732
bokeh/bokeh#13872

Will try to see if I can adapt the code so it works anyway today but if it seems to take too much time I will just workaround the issue by using a previous version (packaging it to npm myself if needed) and wait for them to fix it

After that will take care of the arithmetic columns issues we identified

@francoismg
Copy link
Contributor Author

francoismg commented Sep 24, 2024

I have fixed most of the issues regarding importing bokeh and d3 as npm package however there's still an issue regarding bokeh when migrating from 3.3.* to 3.5.* -> Type error signal is undefined when trying to instantiate a graph

It seems like a known issue or at least related to : bokeh/bokeh#13864 bokeh/bokeh#13732 bokeh/bokeh#13872

Will try to see if I can adapt the code so it works anyway today but if it seems to take too much time I will just workaround the issue by using a previous version (packaging it to npm myself if needed) and wait for them to fix it

After that will take care of the arithmetic columns issues we identified

It's working now but the problem seems to be more related to the use of npm and webpack than the code itself for some reason.

Identical version will work when imported statically and not when imported and built through webpack, apparently some other people are having the same problems.

I had to fallback to version 3.0.3 (previous statical version used was 3.3.2) and manually install a lot of dependencies but it seems ok now. Problem start from version 3.1.0 up to latest 3.5.2.

Maybe best to remember that bokehjs seems to work way better when importing it statically using these :

<script type="text/javascript" src="https://cdn.bokeh.org/bokeh/release/bokeh-x.x.x.min.js"></script>
<script type="text/javascript" src="https://cdn.bokeh.org/bokeh/release/bokeh-gl-x.x.x.min.js"></script>
<script type="text/javascript" src="https://cdn.bokeh.org/bokeh/release/bokeh-widgets-x.x.x.min.js"></script>
<script type="text/javascript" src="https://cdn.bokeh.org/bokeh/release/bokeh-tables-x.x.x.min.js"></script>
<script type="text/javascript" src="https://cdn.bokeh.org/bokeh/release/bokeh-mathjax-x.x.x.min.js"></script>
<script type="text/javascript" src="https://cdn.bokeh.org/bokeh/release/bokeh-api-x.x.x.min.js"></script>

@volodymyrss
Copy link
Contributor

I have fixed most of the issues regarding importing bokeh and d3 as npm package however there's still an issue regarding bokeh when migrating from 3.3.* to 3.5.* -> Type error signal is undefined when trying to instantiate a graph
It seems like a known issue or at least related to : bokeh/bokeh#13864 bokeh/bokeh#13732 bokeh/bokeh#13872
Will try to see if I can adapt the code so it works anyway today but if it seems to take too much time I will just workaround the issue by using a previous version (packaging it to npm myself if needed) and wait for them to fix it
After that will take care of the arithmetic columns issues we identified

It's working now but the problem seems to be more related to the use of npm and webpack than the code itself for some reason.

Identical version will work when imported statically and not when imported and built through webpack, apparently some other people are having the same problems.

I had to fallback to version 3.0.3 (previous statical version used was 3.3.2) and manually install a lot of dependencies but it seems ok now. Problem start from version 3.1.0 up to latest 3.5.2.

Maybe best to remember that bokehjs seems to work way better when importing it statically using these :

<script type="text/javascript" src="https://cdn.bokeh.org/bokeh/release/bokeh-x.x.x.min.js"></script>
<script type="text/javascript" src="https://cdn.bokeh.org/bokeh/release/bokeh-gl-x.x.x.min.js"></script>
<script type="text/javascript" src="https://cdn.bokeh.org/bokeh/release/bokeh-widgets-x.x.x.min.js"></script>
<script type="text/javascript" src="https://cdn.bokeh.org/bokeh/release/bokeh-tables-x.x.x.min.js"></script>
<script type="text/javascript" src="https://cdn.bokeh.org/bokeh/release/bokeh-mathjax-x.x.x.min.js"></script>
<script type="text/javascript" src="https://cdn.bokeh.org/bokeh/release/bokeh-api-x.x.x.min.js"></script>

@burnout87 , does this make sense for you?

@burnout87
Copy link

Identical version will work when imported statically and not when imported and built through webpack, apparently some other people are having the same problems.

This is weird, it seems related to missing dependencies not included when building with webpack

Maybe best to remember that bokehjs seems to work way better when importing it statically using these :

What do you mean with "working better" ? Just that is working as expected?

@francoismg
Copy link
Contributor Author

This is weird, it seems related to missing dependencies not included when building with webpack

I thought so too at first but I have no "module not found error" when building and the "undefined signal" error seems to be related to other issues so I really don't know. Also I tried other versions while keeping the manually installed dependencies and had the same error.

What do you mean with "working better" ? Just that is working as expected?

Yes I meant working as expected and without the need to install other dependencies yourself

@burnout87
Copy link

I had to fallback to version 3.0.3 (previous statical version used was 3.3.2) and manually install a lot of dependencies but it seems ok now. Problem start from version 3.1.0 up to latest 3.5.2.

So, with an older bokeh version, static import is not required right? As you said, I would keep it like that for now, waiting for the fix.

@burnout87
Copy link

bokeh/bokeh#13864 (comment)

And it seems like major work is needed to fix it

@francoismg
Copy link
Contributor Author

I had to fallback to version 3.0.3 (previous statical version used was 3.3.2) and manually install a lot of dependencies but it seems ok now. Problem start from version 3.1.0 up to latest 3.5.2.

So, with an older bokeh version, static import is not required right? As you said, I would keep it like that for now, waiting for the fix.

Yes it's fine with an older version, just some extra dependencies needed in the build but it's working. So ok let's do that and wait for a fix.

@francoismg
Copy link
Contributor Author

Arithmetic columns are now available for error bars. Updated npm package has been published.

Will test again to import the package into another webpack project locally and then to galaxy when it's working

@francoismg
Copy link
Contributor Author

Support for numbers in arithmetic column has been added

Spotted a bug to fix -> deleted custom columns can still appear in dropdown

@francoismg
Copy link
Contributor Author

Integration inside a galaxy plugin is mostly ok, had to make some small changes since it's now a dependency of another webpack app but there seems to be some issues with yarn that cannot find the package during the client build, not sure if it's really related to yarn or just one of the generic issues when trying to build a plugin like with the first version of the plugin

Will make a new branch for the galaxy pull request once the issue is fixed

@francoismg
Copy link
Contributor Author

Build and file load issues inside galaxy have been fixed will make the new branch for pull request and put the link for the new branch here

@francoismg
Copy link
Contributor Author

There seems to have some bugs on galaxy latest dev branch (unrelated to the plugin) since at least yesterday that prevent client build. I pushed the modifications so it's ready for pr but probably better to wait for dev branch to work to make a check before opening the pr

branch is available here: https://github.com/esg-epfl-apc/galaxy/tree/fits-graph-visualization-plugin-pr

@bgruening
Copy link

What is not working on the dev branch? We do have a few green PRs, so it should work.

@francoismg
Copy link
Contributor Author

francoismg commented Oct 9, 2024

What is not working on the dev branch? We do have a few green PRs, so it should work.

There seems to be 2 typescript or vue errors (by forking latest dev or directly cloning https://github.com/galaxyproject/galaxy.git)

TS1484: 'Workflow' is a type and must be imported using a type-only import when 'verbatimModuleSyntax' is enabled

TS2339: Property 'username' does not exist on type '{ isAnonymous: false; deleted: boolean; email: string; id: string; is_admin: boolean; nice_total_disk_usage: string; preferences: Record<string, never>; preferred_object_store_id?: string

Coming from WorkflowInvocationHeader.vue.ts and raised after plugin build when building the whole app frontend (I think)

Build from last week was working fine on the same machine btw

@francoismg
Copy link
Contributor Author

Issues with the latest dev build seem to have been resolved, will update the fork and test it locally

@francoismg
Copy link
Contributor Author

Draft pr is available here galaxyproject/galaxy#19003

Spotted a small issue when working with multiple files and trying to remove a file from plot, was working before so shouldn't be too much trouble to fix, will make pr ready to review after that if it's ok with you

@francoismg
Copy link
Contributor Author

Bug has been fixed, can make the pr ready for review if ok with you

@francoismg
Copy link
Contributor Author

Just spotted some inconsistencies when using user defined arf and rmf files names for spectrum data (sometimes processed columns are not added to available columns for axis), will fix that and then mark galaxy pr ready for review

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.

5 participants