Fix/date context alignment fallback #2060

thoniTUB · 2021-08-31T10:53:36Z

before the fallback for a date context alignment was determined by the order of the hash map. Now an ordered data structure is used there

Njimefo · 2021-08-31T11:32:10Z

backend/src/main/java/com/bakdata/conquery/apiv1/forms/export_form/ExportForm.java

+		if(resolution.supportsAlignment(alignmentHint) ) {
+			return alignmentHint;
+		}
+		return resolution.getDefaultAlignment();


warum nimmst du hier den Default?

@Njimefo
Im ExportFormular kann der Nutzer verschiedene zeitliche Stratifizierungen auswählen, also ob Events über Tage, Quartale oder Jahre zusammen gefasst werden sollen (Monate, Wochen wären theoretisch auch möglich). Das ist die Resolution, also die Auflösung der Stratifizierung.

Dann gibt es noch das kalendarische Alignment. Das gibt an, woran sich die, durch die Resolution festgelegten, Datumsbereiche ausrichten. So kannst du zum Beispiel eine Auflösung von Jahren am heutigen
Tag (31.08.2021 - 31.08.2022),
Quartal (01.07.2021 - 30.06.2022) oder
Jahr (01.01.2021 - 31.12.2021) ausrichten.

Jetzt zum Default: Die API lässt es zu, dass du ein alignmentHint gibts. Es passt aber ein gröberes Aligment nicht zu einer feineren Auflösung: Es ist nicht möglich Quartale am Kalenderjahr auszurichten. An dieser Stelle gibt es einen Fallback zum Default.

awildturtok · 2021-08-31T11:37:30Z

backend/src/main/java/com/bakdata/conquery/apiv1/forms/export_form/ExportForm.java

@@ -132,7 +132,10 @@ public String getLocalizedTypeLabel() {
 	}

 	private static DateContext.Alignment getFittingAlignment(DateContext.Alignment alignmentHint, DateContext.Resolution resolution) {
-		return resolution.getSupportedAlignments().contains(alignmentHint)? alignmentHint : resolution.getSupportedAlignments().iterator().next();
+		if(resolution.supportsAlignment(alignmentHint) ) {


=> isAlignmentSupported

Jep, das ist besser

backend/src/main/java/com/bakdata/conquery/models/error/ConqueryError.java

awildturtok · 2021-08-31T11:50:51Z

backend/src/main/java/com/bakdata/conquery/models/forms/util/DateContext.java

+			// We need to define this in a static block so the runtime can solve this cyclic
+			// dependency between Resolution and alignment. Otherwise a NPE is thrown by the
+			// Collection
+			COMPLETE.compatibleAlignments = List.of(Alignment.NO_ALIGN);


Werde aus dem Kommentar leider nicht schlau, aber warum kannst du das nicht im Konstruktor machen?

Beide kompilieren, aber der Erste läuft auf Grund der NPE nicht:

import lombok.RequiredArgsConstructor; import java.util.List; public class Test { @org.junit.jupiter.api.Test public void test() { E1 e = E1.A; } @RequiredArgsConstructor public static enum E1{ A(List.of(E2.a)); private final List<E2> e; } @RequiredArgsConstructor public static enum E2 { a(List.of(E1.A)); private final List<E1> e; } }

Ohne Collection dazwischen geht es:

import lombok.RequiredArgsConstructor; import java.util.List; public class Test { @org.junit.jupiter.api.Test public void test() { E1 e = E1.A; } @RequiredArgsConstructor public static enum E1{ A(E2.a); private final E2 e; } @RequiredArgsConstructor public static enum E2 { a(E1.A); private final E1 e; } }

awildturtok · 2021-08-31T11:51:25Z

backend/src/main/java/com/bakdata/conquery/models/forms/util/DateContext.java

+			// dependency between Resolution and alignment. Otherwise a NPE is thrown by the
+			// Collection
+			COMPLETE.compatibleAlignments = List.of(Alignment.NO_ALIGN);
+			// Beware that the first alignment is considered the default.


Kannst du das nicht einfach direkt kodieren für ein extra feld?

Ich habe es jetzt als Methode definiert, dann gibt es bei der Konstruktion auch keine cyc Dependenies

awildturtok · 2021-08-31T11:51:50Z

backend/src/main/java/com/bakdata/conquery/models/forms/util/DateContext.java

 		}

 		/**
 		 * Returns the amount of calendar alignment sub date ranges that would fit in to this resolution.
 		 */
 		@JsonIgnore
 		public OptionalInt getAmountForAlignment(Alignment alignment){
-			if (!this.compatibleAlignmentsAndAmount.containsKey(alignment)) {
+			if (!this.compatibleAlignments.contains(alignment)) {


supportsAlignment benutzen.

awildturtok · 2021-08-31T11:53:23Z

backend/src/main/java/com/bakdata/conquery/models/forms/util/DateContext.java

@@ -270,8 +287,24 @@ public OptionalInt getAmountForAlignment(Alignment alignment){
 		QUARTER(CDateRange::getCoveredQuarters),
 		YEAR(CDateRange::getCoveredYears);

+		static {
+			NO_ALIGN.amountPerResolution = Map.of(Resolution.COMPLETE, 1);


Auch hier mir leider nicht klar warum du das nicht im ctor machen kannst.

Frage mich gerade auch, ob das nicht sogar was wäre das schon in einer DatteTime Bibliothek kodiert ist.

Ich glaube nicht, weil es unterschiedliche Definitionen für manches gibt. Im Finanzsektor ist ein Jahr manchmal nur 360 Tage lang.

awildturtok · 2021-08-31T11:54:18Z

backend/src/test/java/com/bakdata/conquery/models/forms/DateContextTest.java

+
+		List<DateContext> contexts = DateContext.generateRelativeContexts(event, indexPlacement, featureTime, outcomeTime, timeUnit, ExportForm.getResolutionAlignmentMap(List.of(YEARS, QUARTERS), YEAR));
+
+		assertThat(contexts).containsExactly (


…ryError.java Co-authored-by: awildturtok <1553491+awildturtok@users.noreply.github.com>

awildturtok · 2021-08-31T14:23:01Z

backend/src/main/java/com/bakdata/conquery/models/forms/util/Alignment.java

+	 */
+	@JsonIgnore
+	public OptionalInt getAmountForResolution(Resolution resolution) {
+		Integer amount = getAmountPerResolution().get(resolution);


Würde ich über contains machen aber unwichtig

Habs angepasst

awildturtok · 2021-08-31T14:24:09Z

backend/src/main/java/com/bakdata/conquery/models/forms/util/CalendarUnit.java

+import lombok.RequiredArgsConstructor;
+
+@RequiredArgsConstructor
+public enum CalendarUnit {


awildturtok · 2021-08-31T14:25:16Z

backend/src/test/java/com/bakdata/conquery/Test.java

+import lombok.RequiredArgsConstructor;
+import java.util.List;
+
+public class Test {


Die sollte jetzt raus sein

thoniTUB added 2 commits August 31, 2021 12:51

use ordered datastructure for Alignment to fallback to the correct one

292ca75

makes Resolution less leaky

c7c561c

thoniTUB requested review from awildturtok and Njimefo August 31, 2021 11:07

Njimefo reviewed Aug 31, 2021

View reviewed changes

Njimefo approved these changes Aug 31, 2021

View reviewed changes

awildturtok requested changes Aug 31, 2021

View reviewed changes

thoniTUB and others added 5 commits August 31, 2021 15:56

refactor Resolution and Alignment class

48fa605

refactor CalendarUnit and AlignmentReference class

f7fbf9c

removes warnings

3042814

Update AutoDoc

33ba59a

remmoves static initializer blocks

aff26e4

thoniTUB requested a review from awildturtok August 31, 2021 14:19

thoniTUB and others added 2 commits August 31, 2021 16:21

Update backend/src/main/java/com/bakdata/conquery/models/error/Conque…

47d6000

…ryError.java Co-authored-by: awildturtok <1553491+awildturtok@users.noreply.github.com>

removes scratch test class

ed6fa31

awildturtok requested changes Aug 31, 2021

View reviewed changes

adds documentation for CalendarUnit

370932f

thoniTUB requested a review from awildturtok August 31, 2021 14:59

awildturtok approved these changes Aug 31, 2021

View reviewed changes

thoniTUB merged commit 2e51801 into release Aug 31, 2021

delete-merged-branch bot deleted the fix/date-context-alignment-fallback branch August 31, 2021 15:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix/date context alignment fallback #2060

Fix/date context alignment fallback #2060

thoniTUB commented Aug 31, 2021

Njimefo Aug 31, 2021

thoniTUB Aug 31, 2021 •

edited

Loading

awildturtok Aug 31, 2021

thoniTUB Aug 31, 2021

awildturtok Aug 31, 2021

thoniTUB Aug 31, 2021

awildturtok Aug 31, 2021

thoniTUB Aug 31, 2021

awildturtok Aug 31, 2021

awildturtok Aug 31, 2021

thoniTUB Aug 31, 2021

awildturtok Aug 31, 2021

awildturtok Aug 31, 2021

thoniTUB Aug 31, 2021

awildturtok Aug 31, 2021

awildturtok Aug 31, 2021

thoniTUB Aug 31, 2021


		List<DateContext> contexts = DateContext.generateRelativeContexts(event, indexPlacement, featureTime, outcomeTime, timeUnit, ExportForm.getResolutionAlignmentMap(List.of(YEARS, QUARTERS), YEAR));

		assertThat(contexts).containsExactly (

Fix/date context alignment fallback #2060

Fix/date context alignment fallback #2060

Conversation

thoniTUB commented Aug 31, 2021

Choose a reason for hiding this comment

thoniTUB Aug 31, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thoniTUB Aug 31, 2021 •

edited

Loading