-
Notifications
You must be signed in to change notification settings - Fork 6
Do not create SVG files when redirected to sdout #287
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
base: main
Are you sure you want to change the base?
Conversation
🚦 Performance Comparison (α=0.05, ±3%)
❌ FailuresNone 😎 🚀 Speed-ups (11)
🐢 Slow-downs (2)
⚖️ No significant change (21)
|
|
Couple of things are needed before I can merge this pull request.
|
adisidev
left a comment
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.
Make changes to pull request comment and code as requested.
While we're at it, see if you can change the following line (in the same pull request) according to the issue described in #243
cart_info.reposition_insets(args.redirect_exports_to_stdout);
…l being created despite the flag's purpose being to redirect all exports to stdout and avoid file creation. This violated the principle that when --redirect_exports_to_stdout is enabled, no files should be written to disk.
nihalzp
left a comment
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.
Thank you for working on the issues. Looks good to me! I think the file sample_data/australia_by_state_and_territory_since_1942/australia_population_by_state_and_territory_2021_cartogram.geojson has been added mistakenly. Once that's removed, we can merge it.
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.
Seems like this file has been mistakenly added.
Modified all SVG file creation functions to check for args_.redirect_exports_to_stdout in addition to their existing plot flags. When --redirect_exports_to_stdout is enabled, SVG files are no longer created regardless of other plot flag settings.
To test:
Test with redirect_exports_to_stdout flag
./cartogram sample_data/world_by_country_since_2022/world_by_country_since_2022.geojson sample_data/world_by_country_since_2022/world_population_by_country_2010.csv --redirect_exports_to_stdout --plot_polygons --add_grid --plot_density --plot_quadtree --plot_intersections
no svg files are created and only geojson output is printed to sdout
@adisidev