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

Add Position and Occupancy for less repetition of position data #1170

Merged
merged 12 commits into from
Jul 21, 2023

Conversation

jbothma
Copy link
Contributor

@jbothma jbothma commented Jul 11, 2023

Adds Position and PositionHeld to eventually replace Post.

Need to fix divergent property type and other issues in CI

@pudo
Copy link
Contributor

pudo commented Jul 11, 2023

Very cool! A few fields I could imagine being useful:

  • Position:inceptionDate
  • Position:dissolutionDate
  • Position:website or Position:sourceUrl ?
  • Position:description ?

Maybe (what's your feeling)?

  • PositonHeld:termStartDate
  • PositonHeld:termEndDate

p.s. I proposed #1167 this morning, which would mark Post as deprecated for a release or two instead of removing it outright. Perhaps you can restore it in this PR so that both discussions are untangled?

@pudo
Copy link
Contributor

pudo commented Jul 11, 2023

Do we want to put our "active/out-of-office/unsure" classification onto the PositionHeld entity?

@jbothma
Copy link
Contributor Author

jbothma commented Jul 11, 2023

p.s. I proposed #1167 this morning, which would mark Post as deprecated for a release or two instead of removing it outright. Perhaps you can restore it in this PR so that both discussions are untangled?

Basing on your branch which deprecates Post makes a lot more sense 🤦

Do you mean restore as in rebase? I rebased and pushed. Would it make sense for me to change the PR base branch to yours so my diff is more focused on the additional changes? Or would that just be more confusing since the intention is to merge separately from your PR.

@jbothma
Copy link
Contributor Author

jbothma commented Jul 11, 2023

Maybe (what's your feeling)?

PositonHeld:termStartDate
PositonHeld:termEndDate

I'd lean more towards creating the term class to hold these dates, rather than representing these dates. many different Positions, e.g. each member of parliament, would repeat the same dates

@jbothma
Copy link
Contributor Author

jbothma commented Jul 11, 2023

Do we want to put our "active/out-of-office/unsure" classification onto the PositionHeld entity?

I like that - it's easy to determine this at data entry and remove the need for data users to do the logic. I left unsure out since I think blank covers that - or is there value in an explicit state to say something like "it's not that we didn't bother setting this, but we really didn't know"

@jbothma
Copy link
Contributor Author

jbothma commented Jul 11, 2023

A few fields I could imagine being useful:

Position:inceptionDate
Position:dissolutionDate
Position:website or Position:sourceUrl ?
Position:description ?

Done. I was thinking those dates would be nice too.

@jbothma
Copy link
Contributor Author

jbothma commented Jul 11, 2023

Is there a nice obvious way to resolve this?

python contrib/check_model.py
DIVERGENT TYPES

  position:
  * Person:position - string
  * PositionHeld:position - entity

Is it just a matter of finding another word for it? I don't want to just throw a thesaurus at it if we're creating a jargon jumble that way

I fixed a message like this for jurisdiction which I initially had as string by changing jurisdiction to country type, and adding jurisdiction_region for subnational in 054cbd3#diff-7280cbef3fa2a8b391edfae34394c47e59ba198befdaad19e1987666c93821e5R48
I do want a country field that makes sense when a diplomat's mandating country and service country/ies are all added. So perhaps country and subnational_region: string are a good balance? I'm not sure the distinction between country relations needs to be machine-readable and it's already evident in the position name

@jbothma
Copy link
Contributor Author

jbothma commented Jul 11, 2023

I still don't know the reality of querying and summarising data in this schema and what the pros/cons are of an enum type for classifications of positions like

national-executive
national-legislature
national-government-administrative
subnational-legislature
party-leadership
diplomatic

as opposed to making it a string field which we control on the OpenSanctions crawler side. Also not sure what the migration path looks like if we want to change from our first decision on this.

The main argument I can think of for enum type is validation of values out of the box (right?).

But perhaps that's something we can do easily enough in OpenSanctions code too, and don't have to weigh down FtM users with our special classification decisions?

@jbothma
Copy link
Contributor Author

jbothma commented Jul 11, 2023

How do you feel about a number_of_seats property? In wikidata that's on the Cabinet, which for us would be the PublicBody. With a better name it might fit on the Position item in FtM, as in n members of the board of Eskom, n members of parliament of south africa. It would be useful for validation against the number of people considered to actively hold the position.

@jbothma jbothma changed the title First quick crack at more flexible position type Add Position and PositionHeld for less repetition of position data Jul 12, 2023
@pudo
Copy link
Contributor

pudo commented Jul 12, 2023

Hey, late session last night :) Some feedback:

a) All FtM props so far at camelCase, let's not start doing snake_case now :)
b) Maybe PositionHeld:position could be .... drum roll for confusion ... PositionHeld:post ? For light de-escalation we have a lot of entity props that don't match the type name, e.g. Ownership:owner, Directorship:director etc.
c) Re the jurisdiction thing I think we should definitely keep country - that's not ambiguous and helpful for rendering the data out. For the sub-unit, I could see jurisdictionArea, territory, subnational all work.
d) I feel really weird about OccupancyStatus as a single-use-data-type. I get what we're trying to do here but this feels like technical debt in FtM right out the gate. Let me think about this for a bit.
e) numberOfSeats or numberOfHolders seems like a good idea on Position.

@pudo
Copy link
Contributor

pudo commented Jul 12, 2023

One more thing: we could also just introduce new prefixes in topics, like pos.subnational that work in conjunction with others (role.pep + pos.subnational).

@jbothma
Copy link
Contributor Author

jbothma commented Jul 12, 2023

One more thing: we could also just introduce new prefixes in topics, like pos.subnational that work in conjunction with others (role.pep + pos.subnational).

Do you mean for high level categorisation?

I like how flexible/powerful combining topics is, but how would we render out useful summaries of positions held if we want something like

  • Office of the National Head government
    • President of X
    • Deputy president of X
    • Assistant to the president of X
  • Office of Subnational Head of government
    • Governor of Y
    • Governor of Z

If that's represented by gov.exec.head and gov.national or gov.state respectively?

@jbothma
Copy link
Contributor Author

jbothma commented Jul 12, 2023

Would we want to represent SOE directorship using these schemata and/or Directorship?
How about private company directorship? If you see OpenSanctions going down that road for PEPs.

Using Directorship instead of PositionHeld means summarisation of PEP data in OpenSanctions, as well as documentation for data users of how to extract why someone is considered a PEP, and exactly what kind of PEP they are, is a bit more complex, but there might be upsides.

@jbothma jbothma marked this pull request as ready for review July 13, 2023 13:24
@pudo pudo requested a review from tillprochaska July 13, 2023 13:42
Comment on lines 35 to 37
category:
label: "Category"
type: string
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure what the use case for category is, but maybe this is already covered by Thing:keywords?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pudo I was supposed to drop category from Position to synthesise it for presentation on the summary page, wasn't I?

@tillprochaska
Copy link
Contributor

@jbothma I added a few comments, mostly from Aleph’s perspective (which probably has slightly different requirements compared to OpenSanctions). As Aleph enables end user search across all schemata, we’re trying to reduce the number of properties. Also, Aleph supports graph and time-based visualizations which is why edge and temporalExtent schema metadata is important to us.

@jbothma jbothma changed the title Add Position and PositionHeld for less repetition of position data Add Position and Occupancy for less repetition of position data Jul 19, 2023
@pudo
Copy link
Contributor

pudo commented Jul 20, 2023

hey @jbothma can you still add the temporalExtent and edge sections (as suggested by @tillprochaska) to the Occupancy so we can merge this very soon?

@jbothma
Copy link
Contributor Author

jbothma commented Jul 20, 2023

can you still add the temporalExtent and edge sections (as suggested by @tillprochaska) to the Occupancy

@pudo I understood the ask for temporal extent to be on position, and edge on occupancy, which are done.

I assumed Occupancy will inherit temporal extent from Interval because it doesn't define different time properties.

Should I still add temporalExtent to occupancy?

@pudo
Copy link
Contributor

pudo commented Jul 20, 2023

No, you're righ!

Make it clearer that it's not Position instances on this end, and keep
it specific enough in case someone wants to model accommodation occupancies
in future.
@jbothma
Copy link
Contributor Author

jbothma commented Jul 21, 2023

I'm happy with this if you folks are!

@pudo pudo merged commit cec9231 into alephdata:main Jul 21, 2023
@jbothma jbothma deleted the positions-for-peps branch July 21, 2023 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants