-
-
Notifications
You must be signed in to change notification settings - Fork 111
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
Add Command Line Interface Wrapper #1256
Conversation
I think to make it easier to change file name and use the same option, I should make path last: e.g. |
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 looks just like what I had in mind! Lightweight, clear, and useful!
Is it too much to ask for the Explorer "code" tab to show both Python code and CLI code to generate the selected plot? :-)
"PR with an MVP of the proposed CLI along with a list of desired but unimplemented features and a list of non-features (things explicitly not considered in scope)."
Can you add such a list to this PR or as a separate issue? I.e. one list of desired future features (with checkboxes indicating they are not implemented), plus another list of things you do not feel belong in an hvPlot CLI (e.g. input filtering and transformation beyond what Explorer does).
Separately from this, I've been thinking that the Explorer should initially usually start with just a hamburger button, without the various selectors visible, and those would show up only when the user clicks on the hamburger. That's independent of this PR, but it would help avoid the user having to make a strict choice between a "plot" and an "explorer"; it would just be a plot that then can be reconfigured if desired. I think that approach works well from the cli, and also in other contexts where the Explorer is used, e.g. Jupyter.
break | ||
|
||
|
||
def parse_inputs(inputs): |
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.
Please add docstrings to this and other functions in this file, stating the intent of the function and the assumptions it makes about its arguments.
Super easy I think.
I think Panel needs a Sidebar card layout that does this. |
Co-authored-by: James A. Bednar <[email protected]>
The issue addressed by this PR started with:
So I did something very simple, I downloaded the first netCDF file I found on https://www.unidata.ucar.edu/software/netcdf/examples/files.html and threw it at the CLI (had to run ![]()
I think this demonstrates that building "an application scientists can easily use to explore datasets" is not as simple as exposing some features of hvPlot in a CLI. I'm overall 0- on this feature, though I won't block it if others want to push for it. (Generally, I'm going to push strongly for fixing hvPlot/HoloViews and documenting them better instead of adding new features, in particular when there is funding to do so). If this CLI makes it in the code base in the end, would it be possible in the future to add another subcommand if we have this need? I'm not sure how |
@maximlt , would your error message have been any different when invoking hvPlot from within Python on the same file? My guess is that the CLI interface makes it very easy to invoke issues that were already present in hvPlot and Explorer itself, making it a useful way to improve testing and usability in general. If a user opens Jupyter, loads that file into xarray, launches the Explorer and gets the same error, that's a usability issue we should be addressing anyway; we just found it much more easily. Of course, some issues will come from the file-loading handling itself, which I argue in #1150 (comment) is a genuine tension but something larger than this specific PR. Making file loading easy and robust is important for HoloViz users and currently not properly localized into a specific library. |
This seems extremely useful, especially if it can be run simply over ssh. The number of co-workers who know about pangeo but still use ncview sometimes is high. One might argue that its an alternative path to lowering the difficulty of using hvplot for simple visualizations. I have no strong opinions about which repo it lives in. |
Over |
Yes tunneling is quite straightforward and common; that's how I used Jupyter notebooks before. https://bluewaters.ncsa.illinois.edu/pythonnotebooks |
No, I can reproduce it with: import hvplot.xarray
import xarray as xr
ds = xr.open_dataset("/Users/mliquet/dev/hvplot/sresa1b_ncar_ccsm3-example.nc")
ds.hvplot.explorer()
The more interfaces the more likely bugs and edge cases are going to be surfaced; I'm not sure that's a good argument to motivate adding more interfaces to a piece of functionality vs. doing better testing. Talking about usability, at the moment the explorer doesn't allow selecting between the variables of an xarray I'll open an issue for each of these problems. |
We haven't managed to reach a consensus on whether this feature belongs to hvPlot or not. @droumis did an amazing job when setting up the HoloViz and per-project governance models, giving us a decision model for this kind of case. Any maintainer can call a vote, let's do that then. @philippjfr @hoxbro @ahuang11 please reply with your vote, either -1 for me:
To precise my vote, I would like to say that it is a temporary -1, I would be open to revisiting this idea in the future. If we vote no, I'd suggest keeping #1150 open to let users interested in this feature chime in/upvote, updating the OP with some more context and the scope of the feature (the issue focused on xarray but the implementation in this PR interfaces with other libraries/file types). |
+1:
However, I'm okay if this lives elsewhere, and actually starting to lean towards Lumen using |
-1 |
On balance I will abstain from the vote and record my vote as -0. I am not the day to day maintainer for hvPlot so the maintenance concerns are not front of mind for me, so I primarily want to respect @maximlt's concerns as lead maintainer. I do see the utility for the CLI particularly for the climate community working with gridded data but would like to work towards supporting that at the Lumen level. |
Oops, I forgot to come back to this! With 2 |
Addresses #1150
Demo:
demo.mp4