Skip to content

GEPS045 and initial attempt at updated places - #806

Closed
prculley wants to merge 5 commits into
gramps-project:masterfrom
prculley:places
Closed

GEPS045 and initial attempt at updated places#806
prculley wants to merge 5 commits into
gramps-project:masterfrom
prculley:places

Conversation

@prculley

@prculley prculley commented Mar 17, 2019

Copy link
Copy Markdown
Contributor

An early upload for GEPS 045: Place Model Enhancements in case anyone wants to see if I'm going in the right direction.

You can create a new SQLite db, import a previous XML into it, show the place tree views, edit places etc.
Other types of import and export are not yet worked on. I expect most reports will work, although this is not yet tested.

A few notes;

  • Place names and place types are actually unified lists, not a primary and alternate list like previously. You can still edit the top item in these lists in the top of the dialogs, changes appear in the displaytabs.
    Users who don't mess with multiple names and types should have no trouble.

  • I made the new editors as much like the old ones as possible, so they should be familiar to previous users.

  • The 266 place types have given me a headache, I think the new place type menus are too much for new users. Try it out to see what I mean. I'm considering a feature to limit the menus to only those types beyond the current 5.0.x small list to the ones actually used. It might make sense to have a preferences item to display them all if I do this. If I do this the users who enter places manually will see only what we have now, if he uses GetGov, he will see additional types as needed for places he has in his db.

  • To deal with the many place types, I am adding the concept of place groups. These groups are used to make the place types menu a bit easier, and to allow selection and display of hierarchies and groups for titling. For example, Gramps originally had the concept of Country, County, City type etc. Since there are now multiple place types which could be described as countries, we use the ADM0 group as a group designation for Countries. The same goes for the other ADMx groups. Some types are also described
    as Religious, or just places.

    The Groups are encoded into the place types as a bit field in the high order bits. This makes identifying a place type as belonging to a group a simple bitwise 'and' operation.

  • I don't feel comfortable with postal codes and some other bits of data that need dates. Nick feels that these should be treated as events, I would prefer to use some type of attribute, possibly extended with date fields. Or maybe just include the dates as attribute notes or in the value field. For the moment, postal codes from previous XML trees are just dropped.

Comments welcomed.

@prculley

Copy link
Copy Markdown
Contributor Author

A potential Place Format Editor.

There are now place hierarchies, so the editor allows the user to select from the many levels of the Administrative hierarchy, the Religious, Geographical, and Civil hierarchy. If he chooses items from more than one hierarchy, I would include include them all, with a separator between them, perhaps ';' or '|'.

I may allow the user to utilize custom hierarchy types in this dialog as well, basically duplicating the Religious/Geographic/Civil sections.

Note that places can sometimes have more that one enclosing place with the same type, for example in the GetGov (GOV database) both the EU and France have place types grouped as ADM0 (basically countries). So I think that the user may sometimes only want to see either the larger, smaller both or none, of similar types of places.

We have added place name abbreviations, so the dialog allows the user to select abbreviations when they are available (I would use the name when not).

All the options makes for a very large dialog, so I put them in a scrollable panel.

Note that at the moment this has no functioning code behind it, so it doesn't show up in the PR.

PlaceFormat

@daleathan

daleathan commented Mar 22, 2019

Copy link
Copy Markdown
Contributor

Note that at the moment this has no functioning code behind it, so it doesn't show up in the PR.

Ok, looks very interesting. By the way before I try testing this should I be concerned that travis-ci failed or should I just ignore that error:

File "/home/travis/build/gramps-project/gramps/gramps/gen/lib/serialize.py", line 63, in __object_hook
setattr(obj, key, value)
File "/home/travis/build/gramps-project/gramps/gramps/gen/lib/place.py", line 413, in set_type
traceback.print_stack(limit=2)

@prculley

Copy link
Copy Markdown
Contributor Author

The changes in the main code mean that the tests are not testing the 'right stuff'. So its not surprising that there are failures. The test code will be updated as one of the last things I do, as that means I won't have to do it quite as many times as other things change.

@SNoiraud

Copy link
Copy Markdown
Member

I can't see something in the place view because I saw some changes in the list view I don't like:
We can't change the first colummn (Name). I used to have the first column set to the last change. Only the place view has this constraint. Why is this so important to have only the name in the first column ?
I think we'll have many bug reports with that.

@SNoiraud

Copy link
Copy Markdown
Member

This is related to the commit cdee00a named:
remove trailing spaces from @Nick-Hall

@SNoiraud

Copy link
Copy Markdown
Member

With the flat view, I get the following:
2019-03-24 10:25:34.827: ERROR: grampsapp.py: line 150: Unhandled exception
Traceback (most recent call last):
File "/home/gramps/prculley/gramps/gui/columnorder.py", line 187, in __on_apply
self.on_apply()
File "/home/gramps/prculley/gramps/gui/views/listview.py", line 512, in set_column_order
self.build_tree(preserve_col=False)
File "/home/gramps/prculley/gramps/gui/views/listview.py", line 321, in build_tree
search=filter_info, sort_map=self.column_order())
File "/home/gramps/prculley/gramps/gui/views/treemodels/placemodel.py", line 261, in init
skip=skip, sort_map=sort_map)
File "/home/gramps/prculley/gramps/gui/views/treemodels/flatbasemodel.py", line 488, in init
self.rebuild_data()
File "/home/gramps/prculley/gramps/gui/views/treemodels/flatbasemodel.py", line 615, in _rebuild_filter
allkeys = self.sort_keys()
File "/home/gramps/prculley/gramps/gui/views/treemodels/flatbasemodel.py", line 572, in sort_keys
for key, data in cursor]
File "/home/gramps/prculley/gramps/gui/views/treemodels/flatbasemodel.py", line 572, in
for key, data in cursor]
File "/home/gramps/prculley/gramps/gui/views/treemodels/flatbasemodel.py", line 478, in
self.sort_func = lambda x: glocale.sort_key(self.smapcol)
File "/home/gramps/prculley/gramps/gui/views/treemodels/placemodel.py", line 184, in sort_change
return "%012x" % data[15]
TypeError: %x format: an integer is required, not list

@prculley

Copy link
Copy Markdown
Contributor Author

I can't see something in the place view because I saw some changes in the list view I don't like:
We can't change the first column (Name). I used to have the first column set to the last change. Only the place view has this constraint. Why is this so important to have only the name in the first column ?
I think we'll have many bug reports with that.

The name column order can be changed in the flat place view, but in the tree view it cannot; this is the same as the people and source tree view and is a consequence of the tree layout.

@prculley

Copy link
Copy Markdown
Contributor Author

With the flat view, I get the following:
File "/home/gramps/prculley/gramps/gui/views/treemodels/placemodel.py", line 184, in sort_change
return "%012x" % data[15]
TypeError: %x format: an integer is required, not list

Fixed. I'm sure there are a lot of bugs left, and I am only getting started on the changes...

@SNoiraud

Copy link
Copy Markdown
Member

Thanks.
I have one question:
In the geoplaces view, we can select the marker color depending on type.
With this extension, what could be the best?
One tab for each category (administration, civil, religious...) or a scrollbar like in the PR #775

@prculley

Copy link
Copy Markdown
Contributor Author

In the geoplaces view, we can select the marker color depending on type.
With this extension, what could be the best?
One tab for each category (administration, civil, religious...) or a scrollbar like in the PR #775

I've already adjusted that marker color tab to be dependent on place type groups, rather than place types. That allows for quite a bit of discrimination. I did not think users would want to select all the way down to specific types, when there were so many of them.
If we decide to make the types visible in menus limited (either manually, or dependent on what is actually used), then it might make a specific type color choice list more reasonable again. At that point I would personally make it a scrolled window of the configuration dialog.

@Nick-Hall

Copy link
Copy Markdown
Member

The use of ADM codes has been discussed previously and rejected. I still have major concerns about the idea of a "fixed" hierarchy.

@prculley

prculley commented Mar 25, 2019

Copy link
Copy Markdown
Contributor Author

Added a first pass of dbapi upgrade. If you have created a new db with this PR prior to this, you will have to delete it and start again, as I didn't get the db version updated until the most recent commit; the 'upgrade' would be erroneously applied to your prior db. But it will upgrade an older dbapi 5.0.x db.

We may want to do some refactoring and a new commit_raw_xxx method that is common to both dbapi and bsddb. As it is, I'm not sure that the method I used for upgrade will work more than once, at least without editing of prior upgrades. This might change a lot if we decide to switch to JSON db... I'm not sure how a schema upgrade would work in that case.

@Nick-Hall I'm not clear on what your objection is about. The hierarchy is no more 'fixed' than it was before. There are no limitations on what can enclose what. The (proposed, potential) place displayer and format editor would simply take advantage of the place groups in the new place type scheme to help decide what to display.

@Nick-Hall

Copy link
Copy Markdown
Member

@prculley I don't see why we need this new concept of a place group. We can already specify a level in the hierarchy. The new editor looks overly cluttered.

As I said, this idea has previously been discussed and rejected. We can discuss it again though. I haven't had time to look at the code thoroughly yet.

@Nick-Hall

Nick-Hall commented Mar 25, 2019

Copy link
Copy Markdown
Member

@prculley What do the "Administrative (General)" and "Places" options do?

Also in the UK a borough comes between county and city (level 4?)

How are custom types grouped?

@prculley

Copy link
Copy Markdown
Contributor Author

@Nick-Hall The Administrative (General) grouping are the types that GOV didn't assign to any of the ADM0-6 groups. Places are types that GOV calls 'Populated places' (building, castle, farm village, town etc.) or Unpopulated place (cemetery, field, ruins etc.), that are not administrative (governmental) entities.

You might want to look at the PR gen.lib.placetype file _MENU to see how GOV is assigning types to groups. Note that some types appear in more than one group. Also note that I made the 'Common' group in the menu as Gramps original types. Or just fire up the PR and try the place type menu.

I looked up the London Borough of Greenwich in GOV and it has a type of Borough (234) which is administrative, but not in one of the ADM0-6 groups. The other type of borough (34) is ADM6, the New York USA boroughs are an example of those. The fact that we have some different types with the same English names is a problem; I've asked our German translators to see if they can find a way to do a better job of translating, so that there is no duplication.

The whole GOV hierarchy is;
Metropolitan Borough of Greenwich (Borough 234) ADM
London Borough of Greenwich (Borough 234) ADM
Greater London (Region 137) ADM
England (Landesteil 216) ADM
United Kingdom of Great Britain and Northern Ireland (kingdom 31) (ADM0 or ADM1)
European Union (confederation 71) ADM0

GeoNames has two Greenwich types; one as ADM3 (roughly counties) and another as PPLX (populated place)

UK  .England.Greater London.Greenwich.London.Greenwich
ADM0.ADM1.   ADM2.          ADM3.     PPLC.  PPLX

However we do this I think there will always be compromises...

@Nick-Hall

Copy link
Copy Markdown
Member

@prculley This hierarchy is totally inappropriate for genealogy research in the UK. The use of ADM codes in this way will be rejected as it was in the past.

@prculley

Copy link
Copy Markdown
Contributor Author

@Nick-Hall I didn't realize Gramps was UK centric.

The place type to place group organization is what GOV provided. It seemed to me to be useful to utilize that organization (It works quite well with the data from US and Europe that I have). But if you won't have it please suggest some other way to organize the 260+ types and data from both GOV and GeoNames.

Whatever we do I think we need to get away from having to edit a text string with very specific characters and syntax; I don't think very many users can deal with that.

P.S. the current place format editor scheme doesn't work well for me with GeoNames data; if I want a reduced place displayer title and I select the two top items (thinking I'd get Country and state), I end up with EU and UK for that area, and USA and Texas for my area.

@Nick-Hall

Copy link
Copy Markdown
Member

@prculley Gramps is not UK centric. It is also not Gov or GeoNames centric.

Why are you persisting with an idea that I had already rejected twice in the past for good reasons and didn't accept for this GEPS? Why not just code the changes that I discussed on the list and accepted.

Please don't consider creating 260 place types. Please also don't put third-party codes in our base lib objects.

I am tempted to reject and close this PR now.

@Nick-Hall

Nick-Hall commented Mar 26, 2019

Copy link
Copy Markdown
Member

Just a quick idea. Why not group the place types by country rather than ADM?

Users could configure their countries. Then you could use the place types in a country in the format editor or even a quick place editor. I think that this was the consensus forming from previous discussions. It could also reduce the size of the place type list. I would be happy with this approach.

I don't really like the existing editor strings either, but it was the only idea that gained support at the time.

@prculley

Copy link
Copy Markdown
Contributor Author

@Nick-Hall I'm sorry, when you edited the 5.1 Roadmap section https://gramps-project.org/wiki/index.php/5.1_Roadmap#Database_model_changes, which includes the words, "deal with 200+ place types" I thought that was an approval to move forward with this.

@Nick-Hall

Copy link
Copy Markdown
Member

P.S. the current place format editor scheme doesn't work well for me with GeoNames data; if I want a reduced place displayer title and I select the two top items (thinking I'd get Country and state), I end up with EU and UK for that area, and USA and Texas for my area.

I get the same problem if I suppress EU and UK ADM0 and ADM1. It would have bad effects on other countries which I also have data in.

GeoNames has no place history either and is very administration based.

@Nick-Hall

Nick-Hall commented Mar 26, 2019

Copy link
Copy Markdown
Member

I'm sorry, when you edited the 5.1 Roadmap section https://gramps-project.org/wiki/index.php/5.1_Roadmap#Database_model_changes, which includes the words, "deal with 200+ place types" I thought that was an approval to move forward with this.

No. I was only agreeing the changes I suggested below. There appears to be a misunderstanding!

I had no idea how we may "deal with 200+ place types". My first instinct is don't. :)

@Nick-Hall

Copy link
Copy Markdown
Member

@prculley I had never intended that we implement all 260 GOV place types. I need to discuss this with Brian Matherly.

@prculley

Copy link
Copy Markdown
Contributor Author

May I respectfully suggest you mark the items on the roadmap in some way to avoid this type of issue in the future. Perhaps 'approved', 'under consideration' and 'rejected'. The 260 place types has been in my proposals since the very beginning, I admit I am guilty of assuming that if we are doing a major part of these enhancements, thew we would be doing them all. Perhaps it would also make sense to edit the GEPS045 page as well, although some of the proposal bits there are clearly already decided.

@ennoborg

Copy link
Copy Markdown
Contributor

Just a quick idea. Why not group the place types by country rather than ADM?

Users could configure their countries. Then you could use the place types in a country in the format editor or even a quick place editor. I think that this was the consensus forming from previous discussions. It could also reduce the size of the place type list. I would be happy with this approach.

Me too. We can't do this developers, because we don't have the knowledge to support all countries, but as a user, I can configure the ones that I'm interested in, including old kingdoms like Prussia, etc. etc.

@Nick-Hall

Nick-Hall commented Mar 26, 2019

Copy link
Copy Markdown
Member

@prculley I have edited the roadmap for clarity. This appears to be a genuine misunderstanding. I was assuming "deal with 200+ place types" would be done with mappings in the import/export code (your recommendation 2 in the GEPS), and saw no need to add a PlaceType section.

@Nick-Hall

Copy link
Copy Markdown
Member

@ennoborg Thanks. You were actually the last person I discussed ADM codes with as far as I can remember.

The country grouping idea is growing on me. We could in fact generate the place types for a country dynamically from existing users data, or let users provide default configurations as you suggest.

These place type lists could be used in the place format editor, and perhaps even for a quick place entry dialog.

@bmatherly

Copy link
Copy Markdown
Contributor

@prculley you are a hero for taking on such a huge endeavor!

Regarding the format dialog, I find scroll bars in a dialog to be cumbersome. Is there any way you could incorporate tabs to break it up?

Also, I don't know what "ADM0" and "ADM6" mean. Do we expect most users to understand that? Maybe we just want to show "States" and not "ADM1" in the UI?

Regarding the 200+ place types... having the giant list of types doesn't bother me as long as the code doesn't need to write special conditionals for each one. Is there any way to put the types in an enum to better manage the mapping?
https://docs.python.org/3/library/enum.html
Maybe that wouldn't help, I don't know.

@prculley

Copy link
Copy Markdown
Contributor Author

@bmatherly Thanks for the 'hero' compliment; approximately 100hrs of work so far, and a lot to go.
Yes I realize that ADM0-ADM6 are not ideal user designations, I had thought to get the idea out there and tweak the names a bit later. But as Nick has just decided NOT to support a more comprehensive place type list within Gramps core, it becomes a moot point. I'll be stripping out a fair bit of code changes and trying to figure out how to map the 200+ types that exist in some of the data sources (Nick's own GetGOV for example) to our small list. I regard the current situation where these types end up as 'custom' GrampsType values, and are not properly translated or maintained for lossless export as not optimum.

But first, life intervenes. I get to take a break to do my taxes...

@Nick-Hall

Copy link
Copy Markdown
Member

@prculley Don't do anything quickly. In light of Brian's comments I am reconsidering my decision.

@Nick-Hall

Nick-Hall commented Mar 27, 2019

Copy link
Copy Markdown
Member

On the 7th Feb, I started a user consultation on possible place enhancements for v5.1. Six proposals were made, but none of them suggested expanding the place types, enhancements place format editor or better GOV integration.

However, two replies did mention the GOV database: one wanted a robust connection to a gazetteer, and another warned of many parts of GOV being only in German.

Some users didn't like Gramps becoming too complicated or bloated. It was pointed out that some new users stop using the product for this reason.

Since users have not had a chance to provide feedback on these latest developments, and because it could be argued that they involve more complexity than the six proposals that were agreed, I think that it is best that we re-open the consultation process and ask users for their opinions on the design of these features.

@gerritv

gerritv commented Mar 27, 2019

Copy link
Copy Markdown

Is it possible to
1- Get an installable copy to try out
2- Can we have 2 versions installed on the same machine? That way I don’t mess up my present efforts.
Interested in trying it out on Dutch places, presently working on second pass through my tree to clean up the mess I made on the first pass :-) (I learned a lot so far)
Gerrit

@Nick-Hall

Copy link
Copy Markdown
Member

@gerritv What operating system are you using?

@gerritv

gerritv commented Mar 27, 2019

Copy link
Copy Markdown

Sorry, forgot to say it is Windows 10. Worst case I can run the trial in a VM

@Nick-Hall

Copy link
Copy Markdown
Member

It is probably easier to run it from source in a VM.

@gerritv

gerritv commented Mar 27, 2019

Copy link
Copy Markdown

Easier for whom? :-)
But Ok, will try that, thx

@bmatherly

Copy link
Copy Markdown
Contributor

I had thought to get the idea out there and tweak the names a bit later.

OK. I'm glad to hear this doesn't have to be the final answer. For myself, I would be disappointed to see "ADM1" end up on the main screen (or even something technical sounding like "state, federal". But if those were in some advanced configuration somewhere that I can ignore, that would probably be OK with me.

I see Nick has reignited the discussion on the users list. In my experience, it is very helpful for people to discuss new features when they have some GUI mockup/example to work from. I really appreciate Nick's commitment to setting expectations with users and getting their feedback. While this creates extra effort, I think it pays dividends in the final product.

@Nick-Hall

Copy link
Copy Markdown
Member

or even something technical sounding like "state, federal"

There are actually two possibilities for this:
7 - Bundesland ("state, federal") and is how we currently translate "State"
130 - Bundesstaat ("Federal State")
At the moment we can't use state beacuse:
34 - Land ("State") meaning nation state
50 - Staat ("Country")

As Paul already mentioned there are two options for borough:
52 - Stadtbezirk - smaller than a city
234 - Borough - larger than a city
and to complicate things Borough is the usual translation for Arrondissement in Canada

and at least two versions of county:
32 - Kreis - used in the USA
20 - Grafschaft - used in the UK

So, the default codes depend on the country.

As a related issue, I'm sure the Dutch don't want to see both "City" and "Town" in the defaults, and the French would like to see "Arrondissement".

@Nick-Hall

Nick-Hall commented Mar 28, 2019

Copy link
Copy Markdown
Member

StoltHD asked an interesting question on the mailing list: 'I can still not understand why Gramps need "Standard Types", is it only because of translation?'

We certainly don't want to burden our translators with 260 strings which may be difficult to translate.

He added that he would be glad to use just custom types.

What are we trying to achieve here? If we just want compatibility with the GOV database, why don't we just store a place type as: id, string, GOV code, grouping. We don't even have to make it a GrampsType.

Why do we need the level of complexity that is being proposed?

@Nick-Hall

Copy link
Copy Markdown
Member

@prculley When I was doing the screenshots, the new dialogs looked good. However, I noticed that some of the tooltips had not been updated from the original dialog. Do you want me to open a bug report?

@prculley

Copy link
Copy Markdown
Contributor Author

@Nick-Hall Regarding tooltips, I still need to take a sweep through the dialogs and try to do better here, no need for bug report (yet...).

Regarding place types as part of Gramps; we could also make a small implementation change to GrampsType; switch CUSTOM from 0 to negative numbers, and UNKNOWN to 0. Then store GOV or other ids in the negative value field. That way when we interchange via Gedcom L extensions or XML we can recover the original GOV ID. I'd still like to assign types to some sort of groups for place displayer/format editor purposes. If we can come up with some sort of way to use the groups as 'anchors' in the place format editor and come up with an acceptable GUI or at least some good defaults for new users. I was originally thinking of putting the 'groups' as high order bits, but an additional field will also work. We would need a new config to let the user adjust the group assignments for his own custom types (at least).

This concept gets the GOV types out of the core Gramps code, although we would still need to have a few conventions on how to store/retrieve the GOV type values.

I've also been considering a possible 'Update GOV data' addon that would sweep through the place data and update at least the types from a MiniGOV downloaded file, this would be used to update current user's GOV based data for whatever type scheme we end up with. It would also allow for changing the language of the type display. I don't recall ever seeing place type data in reports so I don't think we need to support place type translations on the fly.

Food for thought.

@Nick-Hall

Copy link
Copy Markdown
Member

@prculley Yes. That's the right idea. I'm happy with this approach.

We should consider making all types custom types so that they can be moved or removed just like any other type. A set of default types can be loaded when a database is created. Maybe we can ask the users what defaults they would like to see?

I suggest that we start by keeping the existing GOV groups.

The new configuration dialog can be quite simple - just a ListView to display the tree and a few buttons: "+", "-", "Done", "Help". Can we just use drag & drop to move a type between groups? We will need to check that a code is not in use before removing it. When adding a new type the user would just enter a name and code. How would we prevent conflicts with the GOV code range? Would it be possible to provide a small XML file to make adding GOV types easier?

The metadata for place types would just need to contain a list of (code, name, group) values. They could be referenced by just the code.

Tools to update existing user data are always appreciated.

@prculley

Copy link
Copy Markdown
Contributor Author

I've reworked the PR again for more customized place types. I'm closing this one and starting a new one so changes don't get confused.

@prculley prculley closed this Apr 12, 2019
@johnbickmore

Copy link
Copy Markdown
Contributor

See #809 GEPS 045 - a second attempt at enhanced places

@prculley
prculley deleted the places branch March 22, 2025 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants