-
-
Notifications
You must be signed in to change notification settings - Fork 839
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
Exoplanet plugin with QtChart #2447
Conversation
- added a few StelProperties - minor simplifications - preliminary parallel view of old and new plots
Hello @gzotti! Thank you for this enhancement. |
This pull request introduces 1 alert when merging 7e0264a into dea9f56 - view on LGTM.com new alerts:
|
- auto-plotting on all input changes - axis scaling
@alex-w I have now removed the requirement to explicitly press the "Plot" button, it goes whenever some input changes. The only real difference now is the behaviour of QChart for plots with log axes. While QCustomPlot does "something" (I have not found the relevant documentation), QChart plots nothing if at least one value is zero or negative (see metallicity). I am unsure what to do: When creating the series, I could of course filter the values to remove all pairs with zeros/negative numbers if the respective plot axis is set to log. The QCustomPLot is also not free from issues. Switching away from log with negative numbers needs another "Plot" press to show all values. |
This pull request introduces 2 alerts when merging 0189515 into dea9f56 - view on LGTM.com new alerts:
|
Not all data can be plot with log axes - I didn’t added a mistake-proofing feature (a foolproof concept). |
Are you then OK if I remove the old plot and only write in the SUG that "plots with log axes may fail if some values can be 0 or negative"? Else the exclusion scheme may become a bit "messy" ("complex method"). |
Of course, go ahead |
- filter positive-only values for log plots - Plugin dependence on QCustomPlot removed
Before merge, I want to go over the SUG chapter and maybe add/change something about the plots. Else I think the code is OK. |
Hmm... Stellarium crashes at exit |
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.
The Stellarium crashes at exit in master also, so, the issue is not in this PR
@alex-w In the root CMakeLists.txt we require Qt5PrintSupport, "just for qcustomplot". If no longer required, we could remove it, or is it now used by something else? (XLS?). The PrintSky plugin will need it obviously, in case we create a useful plugin, but this is another branch. |
No need Qt5PrintSupport now, you can remove or commented it |
Hello @gzotti! Please check the fresh version (development snapshot) of Stellarium: |
Hello @gzotti! Please check the latest stable version of Stellarium: |
This shall switch the scatter plot in the ExoPlanets plugin from QCustomPlot to QtCharts.
During development, this displays old and new diagrams side-to-side for comparison. Plotting style, colors, axis labels etc. should be identical to the old version.
Description
Fixes: Qt6-inflicted deprecation
Screenshots (if appropriate):
Type of change
How Has This Been Tested?
Test Configuration:
Checklist: