Open-Systems-Pharmacology / OSPSuite.Core

Core functionalities of the Open Systems Pharmacology Suite
Other
5 stars 8 forks source link

Charts: provide more symbol styles? #1789

Closed Yuri05 closed 1 year ago

Yuri05 commented 1 year ago

DevExpress currently provides the following markers (symbol styles) for charts: grafik

However only few of then are supported in PK-Sim/MoBi:

grafik

Any reason why we should not provide ALL available symbol styles?

Yuri05 commented 1 year ago

P.S. How it looks like: https://docs.devexpress.com/CoreLibraries/DevExpress.XtraCharts.MarkerKind#members

msevestre commented 1 year ago

I don't see any.. either arbitrary decision years ago or those were added after the fact It needs to be synchronized with the reporting engine probably so that plots are drawn with the right symbols too

Yuri05 commented 1 year ago

It needs to be synchronized with the reporting engine probably so that plots are drawn with the right symbols too

Yes. tlf already supports a lot of symbols and can be extended with the missing ones (which are "pentagon", "hexagon" and "thin cross" - all others are already in the list) (correct, @pchelle ?) https://github.com/Open-Systems-Pharmacology/TLF-Library/blob/07776aa28f45c662cf77ba7713a1c85cf8c1b3b0/R/utilities-enums.R#L290-L337

PavelBal commented 1 year ago

I am for providing all symbol types... Don't see a reason to artificially limit this.

msevestre commented 1 year ago

doing it now

pchelle commented 1 year ago

It needs to be synchronized with the reporting engine probably so that plots are drawn with the right symbols too

Yes. tlf already supports a lot of symbols and can be extended with the missing ones (which are "pentagon", "hexagon" and "thin cross" - all others are already in the list) (correct, @pchelle ?) https://github.com/Open-Systems-Pharmacology/TLF-Library/blob/07776aa28f45c662cf77ba7713a1c85cf8c1b3b0/R/utilities-enums.R#L290-L337

Yes, reporting engine has an internal dictionary translating input symbol names into tlf symbol names that will need synchronization

msevestre commented 1 year ago

@pchelle can you create an issue for this?