-
Notifications
You must be signed in to change notification settings - Fork 332
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
Slight changes to the report json format #1189
Conversation
One thing I would also like to put to discussion is modifying the distributions to contain a value for each percentile and not for cummulative 10 percentiles. This would enable us the display more detailed distribution graphs |
I would say we could play around with some more fine-grained options for the distribution (1 or 5, for example). In the UI we should always offer an option to toggle back to the old 10-based distribution. |
@Alberth289346 just FYI, this PR will be part of a major release sometimes in the next quarter, where the overhauled report viewer will be released. This might break things for you. |
How much effort would it be to encapsulate the deserialization in the report viewer and also to support legacy reports, i.e., reports generated with the current version prior to this PR? If it is reasonable I would vote for supporting this as the current JSON format was at least somewhat stable throughout the past. |
Not much work. Would be as easy as testing if the json has an attribute called metric and then extracting the values the old way and if it does not exists we would use the new way. But there might be some issues with switching to 100 values in the distribution since we can not extract that amount of detail from the old reports. So any visualization with this detail will not be possible and it will have to be checked whether the old or new format was used |
Also looking at the fact that the current version of the Report Viewer will be the legacy one deployed online (maybe for a long time), I would opt for that. However, only if this does not significantly degrade the code quality and structure of the viewer. Any thoughts on this @tsaglam? |
Good point, I agree. For the distribution we could proceed similarly, if it only has ten values, you only see the old histogram. |
Thanks for the head-up. I had similar thoughts about this issue as sebinside, why is data moved ? It breaks compatibility without getting much in return (data isn't becoming more useful by moving it as far as I can see). I pondered about it, and wrote #1190 to move the discussion away from here. |
core/src/main/java/de/jplag/reporting/reportobject/ReportObjectFactory.java
Show resolved
Hide resolved
core/src/test/java/de/jplag/reporting/reportobject/mapper/MetricMapperTest.java
Show resolved
Hide resolved
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.
Regarding the Java side, this looks good.
core/src/main/java/de/jplag/reporting/reportobject/ReportObjectFactory.java
Show resolved
Hide resolved
…into feature/rework-json
[JPlag Report Viewer] Kudos, SonarCloud Quality Gate passed! |
[JPlag Plagiarism Detector] Kudos, SonarCloud Quality Gate passed! |
This PR applies some slight changes to the format of the overview.json file amd comparison files to make it easier to extend with new Metrics (#1134) and make it fit the format used inside the report viewer.
Changes to the overview.json
The
metric
attribute is removed.A new attribute
distributions
is added. This is a map of each metric as a key (3 Letter abbreviation in upper case e.g. "AVG") and as a key it as a List of numbers as the distribution. This List is identical to the one previously saved in each metric.A new attribute
top_comparisons
is added. This is a list of objects. Each object saves the ids of the two submissions. It also saves a map with each metric as a key and the similarity for this metric as a value.Here is an exampl of how the new overview.json looks compared to the old one. All unchanged attributes values are replaced by
...
:Old:
New:
Changes to the comparison files
similarity
gets replaced bysimilarities
and is now a map of similarities.Old:
New: