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

Already set calendar choice for a date can't be forcefully replaced by the preference one #1777

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

bryndin
Copy link

@bryndin bryndin commented Sep 15, 2024

See https://gramps-project.org/bugs/view.php?id=13403#c67233

This commit introduced a bug
42b71b2#diff-403f8698a9cc1199c93a43229eaeaed780e54ddae595e4fa5942eee5bb88d084

  • I'm not familiar with all the supported calendars, and if the newyear for, e.g. Hebrew, needs to be adjusted. The domain owner's feedback is needed.
  • Text parsing logic to generate a date looks funky. It parses an empty string replacing the previous data object while assuming defaults. It looks suspicious.

@SNoiraud
Copy link
Member

I don't like your reverse change.
In france we always use the gregorian date. When we are into the revolutionary period, it is useful to have the revolutionary calendar.
I never use the date editor except for these revolutionary calendar. This is why this patch has been created.
All my date are entered directy in the date field.

@Nick-Hall Nick-Hall added the bug label Sep 15, 2024
…ites the calendar on the previously saved dates
@bryndin
Copy link
Author

bryndin commented Sep 16, 2024

Looking further into the code, multiple places default to CAL_GREGORIAN. In this case, it's easier to leave it as is and instead fix the gramps/gui/editors/editdate.py to check if it's brand new or the previously set date. Hopefully self.date.dateval == Date.EMPTY check does it.

The side question is the source of truth for the internal data representation. I see Date and a string representation of a date being used (e.g. parsing an empty string to overwrite the empty Date object.

Also, there is a lot of custom parsing and date manipulation logic. Would it be better to use Python's datetime as the source of truth, and have a library of mappers to and from different calendars? Then reuse some logic, e.g. date difference from the lib.

Separately, why not factor out the multi-calendar dealing logic into a separate lib, open-source it, and use that lib inside Gramps instead? I see multiple calendar X <-> calendar Y conversion libs, but no comprehensive solutions.

@bryndin
Copy link
Author

bryndin commented Sep 16, 2024

Updated with a simpler fix, pls take a look @SNoiraud @Nick-Hall

@SNoiraud
Copy link
Member

It's not OK for me.
Effectively, the displayed calendar is the good one like found in the preferences, but months are always gregorian.

@@ -142,6 +142,10 @@ def __init__(self, date, uistate, track):
self.new_year.set_text(self.date.newyear_to_str())

cal = self.date.get_calendar()
# for the brand new dates, use the calendar from user preferences instead of the default Date.CAL_GREGORIAN
if self.date.dateval == Date.EMPTY:
Copy link
Member

@Nick-Hall Nick-Hall Sep 17, 2024

Choose a reason for hiding this comment

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

You should use self.date.is_empty() to check for an empty date.

@SNoiraud
Copy link
Member

In France, I always use the Gregorian calendar. I never use the date editor because I enter the date as 01/01/1900 or January 1, 1900 in the date field.
When I work during the era of revolution, I use the date editor and I want this one to be in "French Revolution" as defined in preference. In this case, I have a lot of revolutionary dates and I still want the date editor to show "French Revolution".
With your patch, it’s now impossible.

@Nick-Hall
Copy link
Member

Also, there is a lot of custom parsing and date manipulation logic. Would it be better to use Python's datetime as the source of truth, and have a library of mappers to and from different calendars? Then reuse some logic, e.g. date difference from the lib.

The python datetime module is limited to the Gregorian calendar and dates in the common era. It also doesn't support date quality or precision.

The Gramps Date object uses an integer to store a serial day number with an epoch in 4713 BC. This allows for easy sorting and date arithmetic. We also store a calendar and the day, month and year. This allows us to display a date without frequent computation of these values.

Separately, why not factor out the multi-calendar dealing logic into a separate lib, open-source it, and use that lib inside Gramps instead? I see multiple calendar X <-> calendar Y conversion libs, but no comprehensive solutions.

Conversion between the SDN and day/month/year is carried out by functions in our gcalendar module. This is open source and based on well known calendrical calculations. New calendars can be added fairly easily.

@Nick-Hall
Copy link
Member

When I work during the era of revolution, I use the date editor and I want this one to be in "French Revolution" as defined in preference.

Do you still want this for dates that have been entered as Gregorian. For example if you edit "1 Jan 1900 Gregorian" it would be converted to "4 Nivôse 108". Although this is the same date, it probably isn't what you want.

Perhaps the suggestion of only using the input calendar for new dates would be an acceptable fix?

@SNoiraud
Copy link
Member

Do you still want this for dates that have been entered as Gregorian. For example if you edit "1 Jan 1900 Gregorian" it would be converted to "4 Nivôse 108". Although this is the same date, it probably isn't what you want.

The French republican calendar is only used between 22 September 1792 and 31 December 1805.
After 1805, it was never used

Perhaps the suggestion of only using the input calendar for new dates would be an acceptable fix?

Perhaps. I don't know. I can test it.

@SNoiraud
Copy link
Member

SNoiraud commented Sep 18, 2024

OK. If we replace
cal = config.get("preferences.calendar-format-input")
self.calendar_box.set_active(cal)

by

if self.date.is_empty():
cal = config.get("preferences.calendar-format-input")
self.calendar_box.set_active(cal)

It works for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants