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

[ENH] Contribute Distortion Index (Olteanu and al. 2019) #142

Open
ceciledebezenac opened this issue Jan 24, 2020 · 6 comments
Open

[ENH] Contribute Distortion Index (Olteanu and al. 2019) #142

ceciledebezenac opened this issue Jan 24, 2020 · 6 comments
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@ceciledebezenac
Copy link

Hello I am just opening this issue following a discussion with @darribas to contribute into segregation the statistic proposed in the following paper:
(Olteanu and al. 2019)
I have a working version in another repository here I and I just wanted to kick off the discussion on how to integrate it in, and have your advice on the process.

@darribas darribas added enhancement New feature or request help wanted Extra attention is needed labels Jan 24, 2020
@ljwolf
Copy link
Member

ljwolf commented Jan 24, 2020

Yes! Rad! This would be wonderful; I'm a big fan of the paper 😁

Taking a look at some small things that might help w/ maintenance, integration, & api consistency, we might want to make a few adjustments? It's real well documented & the code is very clean, so I think it should be simple to change a few things over.

Not sure how @renanxcortes or @knaaptime feel, but I spot these things we'd wanna do for integration/consistency. Renan/Eli, you know better about the actual API of the functions, which we might want to also make consistent w/ the other measures.

  • Use scipy more fully
  • Use pysal where possible
    • Adjacency computations definitely would become a pysal.weights.Rook if when we integrate, or hit the geodata.sindex at least?
  • smaller stuff
    • print generally should use warnings.warn when showing warnings.
    • Maybe we should pull graphics imports inside of the plot methods & try/catch to avoid crashing? Most folks have these installed I think, but it's technically not necessary for the statistical functions, so that might be nice to avoid a hard dependency there?
    • Unittests would be needed
    • documentation/examples should be made generic, so the data would have to be published or examples changed.

@knaaptime
Copy link
Member

agree with Levi's points. also,

  1. is pickle really required? if so, what for?
  2. we have gini and lorenz implementations (but we might prefer your lorenz implementation) so we might want to think about how reconfigure these pieces to fit together without duplication

@knaaptime
Copy link
Member

matplotlib is already a dependency, but might be good to move seaborn into the function

@ceciledebezenac
Copy link
Author

Hello all,
Thank you for going through the code so quickly! I totally agree with all that has been said, I paid very little attention to performance for computing the neighbourhood structure... I used pickle for my own analysis, but left it in the cleaner version for no particular reason.
The scipy.special.kl_div method is great and would make the code a lot simpler. 
Please let me know if there are other parts of it that are obscure...

@ljwolf
Copy link
Member

ljwolf commented Jan 27, 2020

Happy to help get this done :) this would be an awesome addition to the library!

@knaaptime
Copy link
Member

also i just realized seaborn is a dependency elsewhere in the pysal stack (spvcm) so i'd be fine including it as a hard dependency here as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

5 participants