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

Removing python for vsl.plot - WIP #173

Merged
merged 12 commits into from
Oct 17, 2023
Merged

Conversation

ulises-jeremias
Copy link
Member

@ulises-jeremias ulises-jeremias commented Oct 13, 2023

  • Updated vsl.plot to avoid the need of using python
  • Updated vsl.plot to use plotly.js and vweb instead of plotly.py
  • Fixed some CI checks
  • Updated plot examples

Summary by CodeRabbit

New Features:

  • Introduced a more intuitive way to add different types of data visualizations (scatter, pie, heatmap, surface, bar, histogram) to a plot.
  • Added a new App struct to generate an HTML page with a Plotly graph.
  • Introduced a port constant to specify the server port.

Improvements:

  • Renamed the plot module to plotly for clarity.
  • Replaced vsl.errors import with vweb for better error handling.
  • Renamed several functions (e.g., add_trace to scatter3d, add_annotation to annotation, set_layout to layout) for consistency and clarity.
  • Updated examples and tests to reflect the new function names and usage.

Refactor:

  • Removed create_venv and solve_mod_path functions.
  • Updated Trace to represent different trace types.
  • Modified show function to use vweb.run to start the server.

These changes enhance the software's intuitiveness and flexibility, making it easier for users to create and customize their data visualizations.

@coderabbitai
Copy link

coderabbitai bot commented Oct 13, 2023

Rate Limit Exceeded

@ulises-jeremias has exceeded the limit for the number of files or commits that can be reviewed per hour. Please wait 13 minutes and 11 seconds before requesting another review.

How to resolve this issue?

After the wait time has elapsed, convert this PR to a draft and then mark it as ready for review again to re-trigger the review. Alternatively, you can push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per repository.
Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.
Please see our FAQ for further information.

Commits Files that changed from the base of the PR and between 730fb50 and e4587a6.

Walkthrough

This pull request introduces significant refactoring to the plot module, enhancing its modularity and flexibility. It introduces specific trace type functions, renames existing functions for clarity, and updates the server-side rendering. The changes also propagate to the examples and machine learning modules, aligning them with the updated plot module.

Changes

File(s) Summary
plot/show.v Renamed plot to plotly, introduced App struct, added port constant, replaced vsl.errors with vweb, and updated show function to use vweb.run.
plot/plot.v Introduced new functions for adding different types of traces, renamed existing functions for clarity, and improved modularity.
plot/trace.v Added TraceType enum, updated XType, YType, ZType to type aliases, introduced new structs for specific trace types, and updated Trace to be a sum type.
examples/.../main.v Updated to use plot.Plot.new() instead of plot.new_plot(), replaced plt.add_trace() with specific trace type calls, and replaced plt.set_layout() with plt.layout().
ml/kmeans.v, ml/knn.v, ml/linreg.v Updated plot() method to align with the refactored plot module.
plot/plot_test.v Updated test cases to align with the refactored plot module.
examples/io_h5_dataset/main.v, examples/io_h5_relax/main.v Updated import statement from vsl.io.h5 to vsl.inout.h5.

🐇
"In the land of code, where the shadows lie,
A rabbit hopped, its gaze held high.
With every leap, a change was made,
In the realm of plots, a new foundation laid.
Now scatter, pie, and heatmap thrive,
In the garden of traces, they come alive.
So let's celebrate this grand revision,
A leap forward in our coding mission!" 🎉


CodeRabbit is currently paused and will not perform reviews. To resume CodeRabbit reviews, enter @coderabbitai resume

Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • Mention @coderabbitai in any review comment for bot assistance.
  • Note: Review comments are made on code diffs or files, not on the PR overview.

Pause Incremental Reviews

  • Insert @coderabbitai: ignore in the PR description to halt the bot's ongoing reviews. Remove the line to resume.
  • Additionally, enter @coderabbitai pause to halt the bot's ongoing reviews as any issue or review comment. To
    resume reviews, enter @coderabbitai resume as any issue or review comment.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 1

Commits Files that changed from the base of the PR and between caee3db and c72dce1.
Files selected for processing (1)
  • plot/show.v (1 hunks)
Additional comments (Suppressed): 3
plot/show.v (3)
  • 48-54: The show function has been significantly refactored. It now uses vweb.run to start a web server, indicating a shift towards a web-based interface. However, the function only runs if not in test mode. Ensure that this does not affect the testing of this function and that there are alternative ways to test this functionality.

  • 1-12: The imports have been updated to replace vsl.errors and os with vweb, indicating a shift towards web-based operations. A new constant port is defined with the value 8082. Ensure that this port is available and does not conflict with other services.

  • 14-17: The Plot struct is now encapsulated within the App struct, which also includes a vweb.Context field. This suggests a move towards a web application context. Ensure that this change does not affect other parts of the code that use the Plot struct.

plot/show.v Outdated Show resolved Hide resolved
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 1

Commits Files that changed from the base of the PR and between c72dce1 and b92c2fa.
Files selected for processing (1)
  • plot/show.v (1 hunks)
Additional comments (Suppressed): 6
plot/show.v (6)
  • 1-5: The imports os and vsl.errors have been removed and replaced with vweb. This change aligns with the shift from a Python-based approach to a web-based one. However, ensure that the removal of these imports does not affect other parts of the code that might be using them.

  • 7-7: A new constant port has been added, likely to specify the port for the web server. Ensure that this port is not being used by other services to avoid conflicts.

  • 9-12: The Plot struct is now embedded within a new App struct, which also includes a vweb.Context field. This change suggests a shift towards a web-based approach for the plot module. Ensure that all instances of the Plot struct have been updated to reflect this change.

  • 14-41: The index function now returns a vweb.Result instead of !, and it seems to be generating an HTML page with a plot. This is a significant change from the previous functionality. Ensure that this change aligns with the overall design and functionality of the application.

  • 16-17: The json.encode_pretty function is used to encode the plot traces and layout. Ensure that the app.plot.traces and app.plot.layout fields always contain valid data to avoid encoding errors.

  • 43-49: The show function now uses vweb.run to start a web server. This is a significant change from the previous functionality where the function was responsible for creating and displaying the plot. Ensure that this change aligns with the overall design and functionality of the application.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 1

Commits Files that changed from the base of the PR and between b92c2fa and 721d662.
Files selected for processing (1)
  • plot/show.v (1 hunks)
Additional comments (Suppressed): 6
plot/show.v (6)
  • 1-8: The module name has been kept the same, but the import of vsl.errors has been replaced with vweb. A new constant port is introduced, set to 0, which means the server will run on the next available port. This change suggests a shift from using Python for plotting to a V-based solution using the vweb module for server operations.

  • 10-13: The Plot struct is now embedded within a new App struct, which also includes a vweb.Context field. This change allows the application to maintain the plot data and the web context in a single structure, which can be beneficial for managing state and handling requests.

  • 15-42: The index function is updated to return a vweb.Result and generate an HTML page with a Plotly graph. The function encodes the plot data and layout to JSON, then spawns a new function that sleeps for 100 milliseconds before exiting. The HTML page includes a script that imports Plotly, sets the data and layout variables to the encoded JSON, and calls Plotly.newPlot to generate the graph. This change allows the application to generate and display plots directly in a web page, rather than relying on a separate Python environment and shell scripts.

  • 44-50: The show function is modified to use vweb.run to start the server. The function creates a new App instance with the plot data and passes it to vweb.run along with the port. This change simplifies the process of displaying the plot by running a server and serving the plot as a web page.

  • 15-42: The index function is using string interpolation to inject the data and layout variables into the script tag. This could potentially lead to cross-site scripting (XSS) attacks if the data is not properly sanitized. However, as per the instructions, we are assuming that arguments to internal function calls are pre-sanitized and do not require additional error handling.

  • 44-50: The show function only runs the server if not in test mode. Ensure that there is a way to test the server functionality, either by mocking the vweb.run function or by providing a separate test mode for the server.

The create_venv and solve_mod_path functions are removed as part of the refactoring. Ensure that these changes do not affect other parts of the code that might be using these functions.

plot/show.v Outdated Show resolved Hide resolved
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 4

Commits Files that changed from the base of the PR and between 721d662 and 45ebb81.
Files selected for processing (34)
  • examples/data_analysis_example/main.v (4 hunks)
  • examples/fft_plot_example/main.v (3 hunks)
  • examples/plot_annotated_pie_chart/main.v (1 hunks)
  • examples/plot_bar/main.v (1 hunks)
  • examples/plot_basic_heatmap/main.v (1 hunks)
  • examples/plot_bubble_chart/main.v (2 hunks)
  • examples/plot_grouped_bar_chart/main.v (1 hunks)
  • examples/plot_heatmap_golden_ratio/main.v (1 hunks)
  • examples/plot_histogram/main.v (1 hunks)
  • examples/plot_line_plot_with_areas/main.v (3 hunks)
  • examples/plot_pie/main.v (1 hunks)
  • examples/plot_ripple_surface/main.v (1 hunks)
  • examples/plot_saddle_surface/main.v (1 hunks)
  • examples/plot_scatter/main.v (2 hunks)
  • examples/plot_scatter3d_1/main.v (2 hunks)
  • examples/plot_scatter3d_2/main.v (2 hunks)
  • examples/plot_scatter3d_easing/main.v (2 hunks)
  • examples/plot_scatter_colorscale/main.v (2 hunks)
  • examples/plot_scatter_easing/main.v (2 hunks)
  • examples/plot_scatter_with_bars/main.v (2 hunks)
  • examples/plot_scatter_with_histogram/main.v (2 hunks)
  • examples/plot_scatter_with_regression/main.v (3 hunks)
  • examples/plot_shaded_area_sin/main.v (3 hunks)
  • examples/plot_sin_cos_surface/main.v (1 hunks)
  • examples/plot_surface/main.v (1 hunks)
  • examples/plot_surface_easing/main.v (1 hunks)
  • examples/roots_bisection_solver/main.v (1 hunks)
  • ml/kmeans.v (3 hunks)
  • ml/knn.v (2 hunks)
  • ml/linreg.v (2 hunks)
  • plot/plot.v (1 hunks)
  • plot/plot_test.v (1 hunks)
  • plot/show.v (1 hunks)
  • plot/trace.v (2 hunks)
Files skipped from review due to trivial changes (24)
  • examples/fft_plot_example/main.v
  • examples/plot_annotated_pie_chart/main.v
  • examples/plot_bar/main.v
  • examples/plot_basic_heatmap/main.v
  • examples/plot_grouped_bar_chart/main.v
  • examples/plot_heatmap_golden_ratio/main.v
  • examples/plot_histogram/main.v
  • examples/plot_line_plot_with_areas/main.v
  • examples/plot_pie/main.v
  • examples/plot_ripple_surface/main.v
  • examples/plot_scatter/main.v
  • examples/plot_scatter3d_1/main.v
  • examples/plot_scatter3d_2/main.v
  • examples/plot_scatter3d_easing/main.v
  • examples/plot_scatter_easing/main.v
  • examples/plot_scatter_with_bars/main.v
  • examples/plot_scatter_with_histogram/main.v
  • examples/plot_scatter_with_regression/main.v
  • examples/plot_shaded_area_sin/main.v
  • examples/plot_sin_cos_surface/main.v
  • examples/plot_surface_easing/main.v
  • examples/roots_bisection_solver/main.v
  • ml/knn.v
  • ml/linreg.v
Additional comments (Suppressed): 29
examples/plot_surface/main.v (1)
  • 20-30: The refactoring of the plot module and the renaming of the add_trace and set_layout functions to surface and layout respectively have been correctly applied in this hunk. However, ensure that all other instances of add_trace and set_layout in the codebase have been updated to reflect these changes.
examples/plot_scatter_colorscale/main.v (2)
  • 23-27: The new scatter function is more intuitive and specific than the previous add_trace function. However, ensure that all calls to add_trace with trace_type: .scatter throughout the codebase have been updated to scatter.

  • 33-35: The layout function is a more intuitive name than set_layout. However, ensure that all calls to set_layout throughout the codebase have been updated to layout.

plot/plot_test.v (1)
  • 3-13: The changes in the function signature from add_trace to bar and set_layout to layout are reflected correctly in the test. Ensure that all calls to these functions throughout the codebase have been updated to match the new signatures. Also, verify that the new bar and layout functions are implemented correctly and handle the same functionality as the old add_trace and set_layout functions.
examples/plot_bubble_chart/main.v (2)
  • 10-14: The scatter function is missing the marker parameter that was present in the old add_trace function. This parameter is necessary to set the size and color of the markers in the scatter plot. Please verify if this parameter has been moved to another function or if it needs to be added back.
-	plt.scatter(
+	plt.scatter(
 		x: x
 		y: y
 		mode: 'markers'
+		marker: plot.Marker{
+			size: size
+			color: []string{len: x.len * y.len, init: '#FF0000'}
+		}
 	)
  • 21-23: The layout function has replaced the set_layout function. Ensure that all calls to set_layout throughout the codebase have been updated to layout.
examples/plot_saddle_surface/main.v (1)
  • 20-30: The changes in the function calls from add_trace and set_layout to surface and layout respectively are consistent with the changes mentioned in the PR summary. Ensure that these changes are propagated throughout the codebase to maintain consistency.
plot/plot.v (2)
  • 57-60: The add_annotation function has been renamed to annotation. This change is consistent with the renaming of other functions in this module. Ensure that all calls to this function throughout the codebase have been updated to match the new name.

  • 62-65: The set_layout function has been renamed to layout. This change is consistent with the renaming of other functions in this module. Ensure that all calls to this function throughout the codebase have been updated to match the new name.

ml/kmeans.v (3)
  • 150-155: The function set_layout has been renamed to layout. Ensure that all calls to this function throughout the codebase have been updated to match the new function name.

  • 171-175: The function add_trace has been replaced with specific trace type calls. In this case, scatter is used. Ensure that all calls to add_trace throughout the codebase have been updated to match the new function calls.

  • 184-188: The function add_trace has been replaced with specific trace type calls. In this case, scatter is used. Ensure that all calls to add_trace throughout the codebase have been updated to match the new function calls.

plot/show.v (4)
  • 10-13: The App struct now embeds the Plot struct and includes a vweb.Context field. This is a significant change as it introduces a dependency on the vweb module for server operations. Ensure that the vweb module is properly installed and configured in your environment.

  • 15-17: The show function now uses vweb.run to start the server. This is a significant change from the previous implementation which used a shell script to run the plot. Ensure that all calls to this function throughout the codebase have been updated to match the new implementation.

  • 30-42: The index function is responsible for generating the HTML for the plot. It uses the html method of the vweb module to generate the HTML. This is a significant change from the previous implementation which used a shell script to generate the plot. Ensure that all calls to this function throughout the codebase have been updated to match the new implementation.

  • 51-79: The JavaScript code embedded in the HTML is responsible for rendering the plot using the Plotly library. This is a significant change from the previous implementation which used a shell script to render the plot. Ensure that the Plotly library is properly loaded and that the JavaScript code works as expected.

examples/data_analysis_example/main.v (5)
  • 75-81: The new hunk is identical to the old hunk. No changes have been made in this section.

  • 88-105: The add_trace function has been replaced with scatter3d function. Ensure that the new function is correctly implemented and works as expected. Also, the z parameter now takes a 2D array instead of a 1D array. Verify that this change doesn't affect the functionality.

  • 112-115: The set_layout function has been replaced with layout function. Ensure that the new function is correctly implemented and works as expected.

  • 136-146: The add_trace function has been replaced with bar function. Ensure that the new function is correctly implemented and works as expected.

  • 149-151: The set_layout function has been replaced with layout function. Ensure that the new function is correctly implemented and works as expected.

plot/trace.v (8)
  • 1-6: No changes have been made in this hunk. The TraceType enum remains the same.

  • 11-12: The histogram trace type has been added to the TraceType enum. Ensure that the rest of the codebase is updated to handle this new trace type.

  • 15-22: The XType, YType, and ZType type aliases remain the same. No changes have been made.

  • 24-43: The Trace struct has been refactored into a CommonTrace struct. This struct contains common properties that are shared by all trace types. This is a good practice as it reduces code duplication and makes the code more maintainable.

  • 45-143: New structs have been introduced for each trace type. Each struct embeds the CommonTrace struct and adds additional properties specific to that trace type. This is a good practice as it makes the code more modular and easier to understand. However, ensure that all parts of the codebase that use these trace types have been updated to use the new structs.

  • 101-116: The Marker and Line structs remain the same. No changes have been made.

  • 118-124: A new Bins struct has been introduced for bin limits in a histogram trace. Ensure that this struct is used correctly in the rest of the codebase.

  • 127-144: The Trace type has been updated to a sum type representing different trace types. This is a good practice as it makes the code more flexible and easier to understand. However, ensure that all parts of the codebase that use the Trace type have been updated to handle the new sum type.

examples/plot_saddle_surface/main.v Show resolved Hide resolved
plot/plot.v Outdated Show resolved Hide resolved
plot/show.v Outdated Show resolved Hide resolved
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 3

Commits Files that changed from the base of the PR and between 45ebb81 and 3cae3ee.
Files selected for processing (1)
  • plot/show.v (1 hunks)
Additional comments (Suppressed): 2
plot/show.v (2)
  • 17-23: The show method in Plot now uses vweb.run to start the server. This is a significant change from the previous implementation, which used a shell script to run the plot. Ensure that the new implementation is compatible with the rest of the system and that all necessary dependencies are properly handled.

  • 40-92: The index method is introduced to generate the HTML for displaying the plot. This method uses JavaScript to manipulate the plot data and layout, and then uses Plotly to display the plot. This is a significant change from the previous implementation, which used a shell script to run the plot. Ensure that the new implementation is compatible with the rest of the system and that all necessary dependencies are properly handled.

plot/show.v Show resolved Hide resolved
plot/show.v Show resolved Hide resolved
plot/show.v Show resolved Hide resolved
@ulises-jeremias
Copy link
Member Author

@coderabbitai pause

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 5

Commits Files that changed from the base of the PR and between 3cae3ee and 06d845d.
Files ignored due to filter (1)
  • inout/h5/v.mod
Files selected for processing (35)
  • examples/data_analysis_example/main.v (5 hunks)
  • examples/fft_plot_example/main.v (3 hunks)
  • examples/io_h5_dataset/main.v (1 hunks)
  • examples/io_h5_relax/main.v (1 hunks)
  • examples/plot_annotated_pie_chart/main.v (1 hunks)
  • examples/plot_bar/main.v (1 hunks)
  • examples/plot_basic_heatmap/main.v (1 hunks)
  • examples/plot_bubble_chart/main.v (2 hunks)
  • examples/plot_grouped_bar_chart/main.v (1 hunks)
  • examples/plot_heatmap_golden_ratio/main.v (1 hunks)
  • examples/plot_histogram/main.v (1 hunks)
  • examples/plot_line_plot_with_areas/main.v (3 hunks)
  • examples/plot_pie/main.v (1 hunks)
  • examples/plot_ripple_surface/main.v (1 hunks)
  • examples/plot_saddle_surface/main.v (1 hunks)
  • examples/plot_scatter/main.v (2 hunks)
  • examples/plot_scatter3d_1/main.v (2 hunks)
  • examples/plot_scatter3d_2/main.v (2 hunks)
  • examples/plot_scatter3d_easing/main.v (2 hunks)
  • examples/plot_scatter_colorscale/main.v (2 hunks)
  • examples/plot_scatter_easing/main.v (2 hunks)
  • examples/plot_scatter_with_bars/main.v (2 hunks)
  • examples/plot_scatter_with_histogram/main.v (2 hunks)
  • examples/plot_scatter_with_regression/main.v (3 hunks)
  • examples/plot_shaded_area_sin/main.v (3 hunks)
  • examples/plot_sin_cos_surface/main.v (1 hunks)
  • examples/plot_surface/main.v (1 hunks)
  • examples/plot_surface_easing/main.v (1 hunks)
  • examples/roots_bisection_solver/main.v (1 hunks)
  • ml/kmeans.v (3 hunks)
  • ml/knn.v (2 hunks)
  • ml/linreg.v (2 hunks)
  • plot/plot.v (1 hunks)
  • plot/plot_test.v (1 hunks)
  • plot/show.v (1 hunks)
Files skipped from review due to trivial changes (14)
  • examples/data_analysis_example/main.v
  • examples/fft_plot_example/main.v
  • examples/io_h5_dataset/main.v
  • examples/io_h5_relax/main.v
  • examples/plot_basic_heatmap/main.v
  • examples/plot_heatmap_golden_ratio/main.v
  • examples/plot_scatter3d_1/main.v
  • examples/plot_scatter3d_easing/main.v
  • examples/plot_scatter_easing/main.v
  • examples/plot_scatter_with_histogram/main.v
  • examples/plot_surface_easing/main.v
  • examples/roots_bisection_solver/main.v
  • ml/knn.v
  • ml/linreg.v
Additional comments (Suppressed): 58
examples/plot_scatter/main.v (2)
  • 23-27: The function signature for creating a new plot and adding a scatter trace has changed. Ensure that all calls to these functions throughout the codebase have been updated to match the new signatures.

  • 36-38: The function signature for setting the layout of a plot has changed. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.

examples/plot_scatter_colorscale/main.v (2)
  • 23-27: The function signature for creating a new plot and adding a scatter trace has changed. Ensure that all calls to these functions throughout the codebase have been updated to match the new signature.

  • 33-35: The function signature for setting the layout of a plot has changed. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.

examples/plot_scatter3d_2/main.v (2)
  • 18-22: The function signature for creating a new plot and adding a scatter3d trace has changed. Ensure that all calls to these functions throughout the codebase have been updated to match the new signatures.

  • 29-31: The function for setting the layout of the plot has been renamed from set_layout to layout. Verify that all instances of set_layout have been updated to layout in the codebase.

examples/plot_sin_cos_surface/main.v (1)
  • 20-30: The changes in the function calls and the structure of the Plot object seem to be in line with the PR summary. The new function names are more intuitive and the code is more readable. However, ensure that all instances of the old function calls in the codebase have been updated to the new ones. Also, verify that the new surface function behaves as expected and produces the same output as the old add_trace function with trace_type: .surface.
examples/plot_grouped_bar_chart/main.v (2)
  • 10-24: The changes in the function calls from new_plot, add_trace, and set_layout to Plot.new, bar, and layout respectively are consistent with the PR summary. The new function calls are more intuitive and improve the readability of the code. However, ensure that these changes are reflected throughout the entire codebase to maintain consistency.

  • 24-24: The show function call now has an exclamation mark at the end, which in V language is used for error propagation. Ensure that the show function is handling errors correctly and that the calling code is prepared to handle any propagated errors.

examples/plot_annotated_pie_chart/main.v (2)
  • 9-14: The function calls new_plot, add_trace, and set_layout have been replaced with Plot.new, pie, and layout respectively. This change improves readability and makes the code more intuitive. However, ensure that all instances of the old function calls in the codebase have been updated to the new ones to avoid any runtime errors.

  • 18-18: The show function call is now followed by !. This is a V language feature that is used to handle or propagate errors. If show function can return an error, this is a good practice. However, if show function does not return an error, the ! should be removed to avoid unnecessary panic.

examples/plot_bubble_chart/main.v (2)
  • 10-14: The new scatter function is more intuitive and specific than the previous add_trace function. However, ensure that all calls to add_trace in the codebase have been updated to use the new scatter function. Also, verify that the scatter function correctly handles the parameters passed to it.

  • 18-23: The new layout function is more intuitive than the previous set_layout function. However, ensure that all calls to set_layout in the codebase have been updated to use the new layout function. Also, verify that the layout function correctly handles the parameters passed to it.

examples/plot_shaded_area_sin/main.v (3)
  • 10-14: The new scatter function seems to be missing some parameters that were present in the old add_trace function, such as line and name. Ensure that these parameters are handled correctly in the new function.

  • 17-23: The new scatter function is missing the fill, fillcolor, and line parameters that were present in the old add_trace function. Make sure these parameters are handled correctly in the new function.

  • 31-33: The new layout function seems to be missing some parameters that were present in the old set_layout function. Ensure that these parameters are handled correctly in the new function.

examples/plot_histogram/main.v (3)
  • 12-24: The changes in the function calls from new_plot, add_trace, and set_layout to Plot.new, histogram, and layout respectively are consistent with the PR summary. Ensure that these changes are reflected throughout the codebase.

  • 12-24: The new function histogram improves the readability and maintainability of the code by making it clear that a histogram trace is being added. This is a good example of the principle of least astonishment.

  • 12-24: The layout function is more intuitive than the previous set_layout function, as it directly modifies the plot layout. This change improves the readability and maintainability of the code.

examples/plot_saddle_surface/main.v (2)
  • 20-30: The changes in this hunk reflect the refactoring of the plot module. The new_plot function is now accessed as Plot.new, and the add_trace and set_layout functions are replaced with surface and layout respectively. The changes seem to improve the readability and usability of the code. However, ensure that these changes are reflected throughout the codebase to maintain consistency.

  • 30-30: The show function is called with a ! at the end, which in V language is used to handle option types and unwrap the value. If an error occurs, the program will panic. This is considered idiomatic in V for functions that are unlikely to fail or where failure would be a critical error. Ensure that this is the intended behavior.

examples/plot_ripple_surface/main.v (2)
  • 21-31: The refactoring of the plot module to plotly and the renaming of the functions to a more object-oriented style improves the readability and maintainability of the code. The new function names are more intuitive and the code is more modular. However, ensure that all calls to these functions throughout the codebase have been updated to match the new function names and signatures.

  • 21-31: The show function now uses vweb.run to start the server. Verify that the server is correctly started and that the plot is correctly displayed in the browser.

examples/plot_scatter_with_regression/main.v (4)
  • 9-13: The new scatter function is more intuitive and easier to use than the previous add_trace function. However, ensure that all calls to add_trace throughout the codebase have been updated to scatter to match the new function name.

  • 20-23: The scatter function is used again here with a different mode. This is a good use of function overloading, but make sure that the function can handle different modes correctly.

  • 29-31: The layout function is a more intuitive replacement for set_layout. Ensure that all calls to set_layout throughout the codebase have been updated to layout.

  • 32-32: The show function is called with a ! operator, which means it can throw an error. Make sure that this error is handled properly in the calling code.

examples/plot_bar/main.v (3)
  • 6-6: The new_plot function is renamed to Plot.new. Ensure that all calls to this function throughout the codebase have been updated to match the new function name.

  • 8-11: The add_trace function is replaced with specific functions for each trace type. Here, bar is used for adding a bar trace. Ensure that all calls to add_trace with trace_type: .bar are replaced with bar.

  • 12-14: The set_layout function is renamed to layout. Ensure that all calls to this function throughout the codebase have been updated to match the new function name.

examples/plot_scatter_with_bars/main.v (3)
  • 10-14: The new scatter function seems to be missing the marker parameter that was present in the old add_trace function. Ensure that the marker settings are still being applied correctly.
-	plt.scatter(
+	plt.scatter(
+		marker: plot.Marker{
+			size: []f64{len: x.len, init: 10.0}
+			color: []string{len: x.len, init: '#FF0000'}
+		}
  • 20-26: Similar to the scatter function, the bar function is also missing the marker parameter. Make sure to include it to maintain the same functionality as before.
-	plt.bar(
+	plt.bar(
+		marker: plot.Marker{
+			color: []string{len: x.len, init: '#0000FF'}
+		}
  • 27-29: The layout function has replaced the set_layout function. Ensure that all instances of set_layout have been replaced with layout throughout the codebase.
examples/plot_line_plot_with_areas/main.v (3)
  • 11-15: The new scatter function seems to be missing the line parameter that was present in the old add_trace function. This parameter allowed to specify the color of the line. Ensure that this functionality is not lost in the new implementation.
-	plt.scatter(
+	plt.scatter(
		x: x
		y: y1
		mode: 'lines'
+		line: plot.Line{
+			color: '#FF0000'
+		}
	)
  • 21-24: Similar to the previous comment, the scatter function call for 'cos(x)' is missing the line parameter. Ensure that the color specification is not lost.
-	plt.scatter(
+	plt.scatter(
		x: x
		y: y2
		mode: 'lines'
+		line: plot.Line{
+			color: '#0000FF'
+		}
	)
  • 30-32: The function set_layout has been renamed to layout. Ensure that all calls to this function throughout the codebase have been updated to match the new function name.
examples/plot_pie/main.v (4)
  • 6-6: The new function is now a method of the Plot struct. Ensure that all calls to this function throughout the codebase have been updated to match the new usage.

  • 8-13: The add_trace function is replaced with specific functions for each trace type. Here, plt.pie is used instead of plt.add_trace. Ensure that all calls to add_trace throughout the codebase have been updated to match the new usage.

  • 14-18: The set_layout function is renamed to layout. Ensure that all calls to set_layout throughout the codebase have been updated to match the new usage.

  • 19-19: The show function is now a method of the Plot struct. Ensure that all calls to this function throughout the codebase have been updated to match the new usage.

examples/plot_surface/main.v (2)
  • 20-30: The refactoring of the plot module to plotly and the renaming of the functions have been correctly applied in this hunk. The new function names and structure improve readability and make the code more intuitive. However, ensure that all calls to these functions throughout the codebase have been updated to match the new function names and signatures.

  • 20-30: The show function now uses vweb.run to start the server. Verify that the server is correctly set up and running as expected. Also, ensure that the port constant is correctly set and used.

plot/plot_test.v (4)
  • 4-4: The function new_plot() has been renamed to Plot.new(). Ensure that all calls to this function throughout the codebase have been updated to match the new function name.

  • 6-9: The function add_trace() has been replaced with specific functions for each trace type. In this case, bar() is used. Ensure that all calls to add_trace() throughout the codebase have been updated to match the new function names.

  • 10-12: The function set_layout() has been renamed to layout(). Ensure that all calls to this function throughout the codebase have been updated to match the new function name.

  • 13-13: The show() function now uses vweb.run to start the server. Ensure that this change does not affect the functionality of the code and that the server starts as expected.

ml/kmeans.v (3)
  • 150-155: The function call plt.layout is updated from plt.set_layout. Ensure that all instances of plt.set_layout in the codebase are updated to plt.layout to maintain consistency.

  • 171-175: The function call plt.scatter is updated from plt.add_trace. The trace_type argument is removed and the function name is changed to reflect the specific type of trace being added. Ensure that all instances of plt.add_trace with trace_type: .scatter in the codebase are updated to plt.scatter.

  • 184-188: The function call plt.scatter is updated from plt.add_trace. The trace_type argument is removed and the function name is changed to reflect the specific type of trace being added. Ensure that all instances of plt.add_trace with trace_type: .scatter in the codebase are updated to plt.scatter.

plot/plot.v (7)
  • 13-15: The function Plot.new() is a good replacement for the old new_plot() function. It's more idiomatic to V and improves readability.

  • 17-22: The add_trace function has been made generic, which is a good change. It allows for different types of traces to be added to the plot without having to modify the function itself.

  • 24-46: The addition of specific functions for adding different types of traces (scatter, pie, heatmap, surface) is a good design choice. It makes the API more explicit and easier to use.

  • 48-59: The scatter3d function is a good replacement for the old add_trace function when the trace type is scatter3d. The flattening of the z value if it's a 2D array is preserved from the old code.

  • 61-71: The addition of functions for adding bar and histogram traces is a good design choice. It makes the API more explicit and easier to use.

  • 73-78: The annotation function is a good replacement for the old add_annotation function. It's more idiomatic to V and improves readability.

  • 80-85: The layout function is a good replacement for the old set_layout function. It's more idiomatic to V and improves readability. The check for the layout range length is preserved from the old code.

plot/show.v (2)
  • 18-23: The show function now starts a server using vweb.run. This is a significant change from the previous implementation, which wrote the plot data to a JSON file and executed a shell script to display the plot. Ensure that this change doesn't break any existing functionality that depends on the old behavior.

  • 42-53: The plotly function is responsible for generating the HTML page that displays the plot. It uses the encode function to convert the plot data and layout to JSON, and then embeds the JSON in a script tag in the HTML. This is a different approach from the previous implementation, which saved the plot data and layout to JSON files and ran a shell script to display the plot. Ensure that this new approach doesn't introduce any security vulnerabilities, such as cross-site scripting (XSS) attacks.

examples/plot_bar/main.v Show resolved Hide resolved
@@ -2,18 +2,19 @@ module main

import vsl.plot
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The import statement is not updated to reflect the new module name plotly. It should be import vsl.plotly instead of import vsl.plot.

- import vsl.plot
+ import vsl.plotly

plot/show.v Show resolved Hide resolved
plot/show.v Show resolved Hide resolved
plot/show.v Show resolved Hide resolved
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 0

Commits Files that changed from the base of the PR and between 06d845d and 730fb50.
Files selected for processing (1)
  • examples/plot_histogram/main.v (1 hunks)
Additional comments (Suppressed): 3
examples/plot_histogram/main.v (3)
  • 9-11: The error handling in the random number generation has been simplified. Ensure that this change does not affect the expected behavior of the program.

  • 12-20: The add_trace function has been replaced with specific trace type calls. This change improves readability and makes the code more intuitive. However, ensure that all instances of add_trace in the codebase have been replaced with the appropriate trace type calls.

  • 21-24: The set_layout function has been renamed to layout. This change simplifies the function name and makes it more intuitive. However, ensure that all instances of set_layout in the codebase have been replaced with layout.

@ulises-jeremias
Copy link
Member Author

@coderabbitai pause

@ulises-jeremias ulises-jeremias added the Hacktoberfest This label is assigned to any issue that is good to go for any Hacktoberfest participant label Oct 17, 2023
@ulises-jeremias
Copy link
Member Author

ulises-jeremias commented Oct 17, 2023

#115
#78
#76

related

@ulises-jeremias ulises-jeremias removed the Hacktoberfest This label is assigned to any issue that is good to go for any Hacktoberfest participant label Oct 17, 2023
@ulises-jeremias ulises-jeremias merged commit 3d7569a into main Oct 17, 2023
@ulises-jeremias ulises-jeremias deleted the feature/remove-python branch October 17, 2023 02:15
@ulises-jeremias ulises-jeremias added Hacktoberfest This label is assigned to any issue that is good to go for any Hacktoberfest participant hacktoberfest-accepted labels Oct 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Hacktoberfest This label is assigned to any issue that is good to go for any Hacktoberfest participant hacktoberfest-accepted
Projects
None yet
2 participants