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

Timeline Chart - add grouping separator #1403

Merged
merged 1 commit into from Feb 28, 2020

Conversation

Pnda87
Copy link
Contributor

@Pnda87 Pnda87 commented Feb 27, 2020

Im Timeline Chart werden Zahlen ohne Tausender Trennzeichen dargestellt. Das wurde hier korrigiert.

Vorher:
vorher

Nachher
nachher

PS: mein erster pull request.
Wenn ich den Commit in Eclipse ausführe funktioniert es. Wenn ich jedoch mit Maven eine exe baue werden garkeine Zahlen angezeigt. Was mache ich da beim build falsch?

VG
Martin

@buchen
Copy link
Member

buchen commented Feb 28, 2020

Super! Ich habe mich da in der Vergangenheit schon dran versucht, aber irgendwas muss ich falsch gemacht haben...

@buchen buchen merged commit be84604 into portfolio-performance:master Feb 28, 2020
@buchen
Copy link
Member

buchen commented Feb 29, 2020

@Pnda87 Ich habe Deine Änderung jetzt bei mir ausprobiert. Bei mir werden jetzt überhaupt keine Zahlen mehr auf der Y-Achse angezeigt (siehe Screenshot). Wenn ich Deine Änderung wieder "revert"e, dann sind die Zahlen wieder da - aber eben ohne Tausendertrennzeichen.

Ich frage mich was da das Problem ist. Das kann doch keine Windows vs. macOS Sache sein?!?

Können wir mal unsere OS und NL Parameter vergleichen?

java.version: 11.0.4
java.vm.version: 11.0.4+11-LTS
os.arch: x86_64
os.name: Mac OS X
os.version: 10.15.3
osgi.nl: de_DE
osgi.nl.user: de_DE
user.country: DE
user.language: de

Bildschirmfoto 2020-02-29 um 07 03 10

buchen added a commit that referenced this pull request Feb 29, 2020
@Pnda87
Copy link
Contributor Author

Pnda87 commented Feb 29, 2020

Hallo @buchen
Hm, sehr seltsam. Ich hatte das Problem oben ja auch schon angesprochen (unter meinen Screenshots). Es trat bei mir auf, wenn ich mit Maven eine ausführbare exe Datei gebaut habe. Es werden dann keine Zahlen < 1 Mio auf der Y-Achse angezeigt (bis auf die "0"), genau wie bei dir. Zahlen größer 1 Mio sind kein Problem:
Zahlen_groesser_1Mio

Wenn ich den selben Codestand über die Eclipse IDE starte tritt das Problem nicht auf. Alle Zahlen werden korrekt im Chart angezeigt.

Ich verstehe nicht woran das liegt. Hast du eine Idee?

Wo kann ich die Parameter denn am einfachsten auslesen?
Im Error Log aus der Version, die ich über die Eclipse IDE gestartet habe, steht folgendes:

!SESSION 2020-02-29 08:48:27.743 -----------------------------------------------
eclipse.buildId=unknown
java.version=9.0.4
java.vendor=Oracle Corporation
BootLoader constants: OS=win32, ARCH=x86_64, WS=win32, NL=en_US

@Pnda87
Copy link
Contributor Author

Pnda87 commented Feb 29, 2020

Ich weiß zwar nicht, ob es unser Problem lösen würde, aber:
In dem Zusammenhang ist mir aufgefallen, dass eine alte Version des swtchart Plugins verwendet wird
swtchart_alteversion

Wenn ich das richtig sehe, wurde das Plugin als Eclipse project refactored:
https://projects.eclipse.org/projects/science.swtchart

Ich scheitere aber irgendwie, das korrekt einzubinden.

@buchen
Copy link
Member

buchen commented Mar 1, 2020

Wenn ich das richtig sehe, wurde das Plugin als Eclipse project refactored:
https://projects.eclipse.org/projects/science.swtchart
Ich scheitere aber irgendwie, das korrekt einzubinden.

Ja, ich weiß es wird einen neue Version geben. Da gibt es ein paar inkompatible API Änderungen. Ich habe lokal schon einen Branch rumliegen mit ersten Anpassungen. Den habe ich jetzt mal auch hier auf Github hochgeschoben: https://github.com/buchen/portfolio/tree/feature_org.eclipse.swtchart. Aber wenn ich das sehe ist die letzte veröffentlichte Version immer noch Version 0.7. Sobald es eine Update Site mit einer offiziellen Version (geplant ist wohl Version 0.12) gibt, mache ich mich an das Update.

Wenn ich den selben Codestand über die Eclipse IDE starte tritt das Problem nicht auf. Alle Zahlen werden korrekt im Chart angezeigt.

Bei mir tritt das Problem auch in der IDE auf. Starte doch mal PP aus der IDE (also mit funktionierenden Axis Tick) und kopiere die vollständigen Installationsdetails (Hilfe -> Über Portfolio Performance -> Installationsdetails) und schicke dir mir an *portfolio dot performance dot help at gmail dot com). Vielleicht finden wir so wo der Unterschied ist. Du könntest es auch mit Deinem Maven Build vergleichen (der ja nicht tut) und schauen ob Du relevante Deltas findest.

@buchen
Copy link
Member

buchen commented Mar 1, 2020

Ich habe das Problem gefunden.

SWTChart ändert die Visibility des Tick Labels auf "false" wenn es das Label nicht zu dem Double value parsen kann. Da wird dann aus dem Label "50.000" (deutsches Format) eine "50" double value (vermutlich mit englischen Format geparst). Ich muss schauen ob ich das "von außen" fixen kann.

https://github.com/eclipse/swtchart/blob/702a9ae2705485f3eeb85e02b60dbd9f2f4df894/org.eclipse.swtchart/src/org/eclipse/swtchart/internal/axis/AxisTickLabels.java#L414

@buchen
Copy link
Member

buchen commented Mar 1, 2020

@Pnda87 - ich habe mal einen Patch vorgeschlagen: eclipse/swtchart#121 Damit tut es dann korrekt.

Bis ich org.eclipse.swtchart eingebunden haben, werden ich Deinen Patch hier reverten. Ich komme an die Klassen nicht dran - und für ein Label lohnt sich der Aufwand nicht.

buchen added a commit that referenced this pull request Mar 1, 2020
@Pnda87
Copy link
Contributor Author

Pnda87 commented Mar 1, 2020

Hallo @buchen
danke für deine Mühe und deine ausführlichen Erklärungen. Alles soweit nachvollziehbar.

Ich werde deinen swtchart branch weiter beobachten und mich dann ggfs. dort auch einbringen. Der Trennzeichen Fix muss dann wohl noch etwas warten.

buchen added a commit that referenced this pull request Mar 4, 2020
If the label cannot be converted back to a number using Double.parseDouble
the label is shown and we can apply formatting. The 'k' prevents the
parsing. And we save some screen space because the labels are shorter.

Issue: #1403
@buchen
Copy link
Member

buchen commented Mar 4, 2020

@Pnda87 mir ist noch eine Idee eingefallen: Wenn man das Label als "100k" für 100.000 formatiert, kann SWTChart das Label nicht per Double.parseDouble umwandeln und zeigt das Label an. Und es schafft auch Platz weil die Labels kürzer sind. Siehe 62e9c95

BTW, SWTChart plant für Mitte März eine Release.

@Pnda87
Copy link
Contributor Author

Pnda87 commented Mar 6, 2020

@buchen sehr elegante Lösung! Finde die Idee an sich gut.

Mir ist allerdings noch aufgefallen: Das Problem besteht bei Beträgen von 1 Mio in veränderter Form weiter:
TimelineChart_1000k

hattest du es bei den Charts der einzelnen Wertpapiere absichtlich weggelassen? Hier z.B. der DowJones
chart_wertpapiere

@buchen
Copy link
Member

buchen commented Mar 7, 2020

Mir ist allerdings noch aufgefallen: Das Problem besteht bei Beträgen von 1 Mio in veränderter Form weiter

Stimmt. Das fixe ich.

hattest du es bei den Charts der einzelnen Wertpapiere absichtlich weggelassen? Hier z.B. der DowJones

Ja. Wenn ich die Kurse mit "k" formatiere, dann ist das (häufig) eben 0,032k und das sieht blöd aus. Klar, der Dow Jones könnte das vertragen, aber die meisten anderen Wertpapiere nicht.

buchen added a commit that referenced this pull request Mar 7, 2020
cmaoling pushed a commit to cmaoling/portfolio that referenced this pull request Apr 12, 2020
cmaoling pushed a commit to cmaoling/portfolio that referenced this pull request Apr 12, 2020
cmaoling pushed a commit to cmaoling/portfolio that referenced this pull request Apr 12, 2020
If the label cannot be converted back to a number using Double.parseDouble
the label is shown and we can apply formatting. The 'k' prevents the
parsing. And we save some screen space because the labels are shorter.

Issue: portfolio-performance#1403
cmaoling pushed a commit to cmaoling/portfolio that referenced this pull request Apr 14, 2020
cmaoling pushed a commit to cmaoling/portfolio that referenced this pull request Apr 14, 2020
cmaoling pushed a commit to cmaoling/portfolio that referenced this pull request Apr 14, 2020
If the label cannot be converted back to a number using Double.parseDouble
the label is shown and we can apply formatting. The 'k' prevents the
parsing. And we save some screen space because the labels are shorter.

Issue: portfolio-performance#1403
cmaoling pushed a commit to cmaoling/portfolio that referenced this pull request Apr 14, 2020
chrisaut pushed a commit to chrisaut/portfolio that referenced this pull request Jun 12, 2020
@Pnda87 Pnda87 deleted the TimelineChart branch September 3, 2022 12:22
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.

None yet

2 participants