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.

Sign up for GitHub

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

Jump to bottom

#1750 bad show of csv #1890

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from
Open

#1750 bad show of csv #1890

wants to merge 3 commits into from

Conversation

foralost
Copy link
Contributor

Converting values to respective Doubles from Ints (making the dot 'visible').
Adding Units column do Header.csv.

Closes #1750.

@foralost foralost force-pushed the #1750_Bad_show_of_csv branch 2 times, most recently from 7a6873e to da1be88 Compare December 10, 2023 21:54
foralost
foralost commented Dec 10, 2023
View reviewed changes
uhabits-core/src/jvmMain/java/org/isoron/uhabits/core/io/HabitsCSVExporter.kt Outdated
@@ -124,7 +124,7 @@ class HabitsCSVExporter(
val dateFormat = DateFormats.getCSVDateFormat()
for ((timestamp, value) in entries.getKnown()) {
val date = dateFormat.format(timestamp.toJavaDate())
out.write(String.format(Locale.US, "%s,%d\n", date, value))
out.write(String.format(Locale.US, "%s,%.3f\n", date, value.toDouble() / 1000.0))
Copy link
Contributor Author

@foralost foralost Dec 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a quite dangerous move to use %.3f here. The better approach would be to use opencsv library, to a have a somewhat standardized approach with these things (the dot in fraction point might be some headache maker).

Did not do it, because it would need a separate MR definitively.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... also the 1000 thing should be standardized somehow, cause it could be often forgotten using this value.

@iSoron
Copy link
Owner

iSoron commented Jan 31, 2024

Thanks for the PR, @foralost. I few comments:

  1. Simply dividing the value by 1000 does not work; this breaks YES/NO habits.
  2. Note that some values for numerical habits have special meaning. For example, -1 means UNKNOWN. You can see the complete list in the class Entry.
  3. Currently the app prints numerical values in the CSV file for YES/NO habits. It would be much better if we printed their friendly names (YES_MANUAL, YES_AUTO, NO, etc).
  4. Besides unit, we are missing a few additional columns, such as notes and numerical targets.

@foralost
Copy link
Contributor Author

foralost commented Feb 7, 2024

I will look & test it in the next week to finish this PR

@foralost
Copy link
Contributor Author

Ready for review.

@foralost
Copy link
Contributor Author

foralost commented Feb 20, 2024 •
edited
Loading

jesus

are there also .csv tests?
maybe i forgot to update them

Yes, there are ;)

will look today

@foralost
Copy link
Contributor Author

foralost commented Feb 20, 2024 •
edited
Loading

ok, i setup whole env on ubuntu (previously worked on Windows).

I am still fighting with testExportCSV for whatever reason (the temp file is empty???)

However the DateUtils test is failing on dev:

getWeekdaysInMonth[jvm]
java.lang.AssertionError: 
Expected: [<4>, <4>, <4>, <4>, <5>, <5>, <5>]
     but: was [<4>, <4>, <4>, <4>, <4>, <4>, <4>]
	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:6)
	at org.isoron.uhabits.core.utils.DateUtilsTest.getWeekdaysInMonth(DateUtilsTest.kt:128)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.mockito.internal.runners.DefaultInternalRunner$1$1.evaluate(DefaultInternalRunner.java:55)
	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
	at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
	at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
	at org.mockito.internal.runners.DefaultInternalRunner$1.run(DefaultInternalRunner.java:100)
	at org.mockito.internal.runners.DefaultInternalRunner.run(DefaultInternalRunner.java:107)
	at org.mockito.internal.runners.StrictRunner.run(StrictRunner.java:42)
	at org.mockito.junit.MockitoJUnitRunner.run(MockitoJUnitRunner.java:163)
	at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.runTestClass(JUnitTestClassExecutor.java:112)
	at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.execute(JUnitTestClassExecutor.java:58)
	at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.execute(JUnitTestClassExecutor.java:40)
	at org.gradle.api.internal.tasks.testing.junit.AbstractJUnitTestClassProcessor.processTestClass(AbstractJUnitTestClassProcessor.java:60)
	at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.processTestClass(SuiteTestClassProcessor.java:52)
	at jdk.internal.reflect.GeneratedMethodAccessor12.invoke(Unknown Source)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
	at org.gradle.internal.dispatch.ContextClassLoaderDispatch.dispatch(ContextClassLoaderDispatch.java:33)
	at org.gradle.internal.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:94)
	at com.sun.proxy.$Proxy2.processTestClass(Unknown Source)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker$2.run(TestWorker.java:176)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.executeAndMaintainThreadName(TestWorker.java:129)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:100)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:60)
	at org.gradle.process.internal.worker.child.ActionExecutionWorker.execute(ActionExecutionWorker.java:56)
	at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:113)
	at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:65)
	at worker.org.gradle.process.internal.worker.GradleWorkerMain.run(GradleWorkerMain.java:69)
	at worker.org.gradle.process.internal.worker.GradleWorkerMain.main(GradleWorkerMain.java:74)

I will resume playing with it probably on Sunday.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Bad show of logs on .CSV when I exported my habits
2 participants
@foralost @iSoron