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

Added __repr__ to Moran I #527

Closed
wants to merge 1 commit into from
Closed

Added __repr__ to Moran I #527

wants to merge 1 commit into from

Conversation

jlaura
Copy link
Member

@jlaura jlaura commented Oct 23, 2014

While the memory address returned is highly informative....I thought that it would be nicer to see a better representation of the class. Perhaps this is a template for other classes?

Here is an example, generated on a network using GeoDaNet Crimes data and counts per edge.

#Moran's I
res = ps.esda.moran.Moran(y, ntw.w, permutations=99)
print res


        Moran's I: 0.00519268749608

        Normality Assumption
        --------------------
        Expected Value: -0.00331125827815
        Variance of I: 0.001848680825
        Standard Dev. of I: 0.0429962885026
        Z-Value: 0.197783252239
        P-Value: 0.843214650106

        Randomization Assumption
        ------------------------
        Variance of I: 0.00184235970473
        Standard Dev. of I: 0.0429227178162
        Z-Value: 0.198122258023
        P-Value: 0.842949410612

        Permutation Results
        -------------------
        Number of Permutations: 99
        Average Expected Value: -0.00471588766906
        Variance of I: 0.000802558883639
        Standard Dev. of I: 0.0283294702322
        Standardized I: 0.349762105818
        P-Value: 0.363258620035

@sjsrey
Copy link
Member

sjsrey commented Oct 23, 2014

I like the idea of having more informative repr . We probability should think through a consistent look/style for all of PySAL. Do we want to use repr and/or summary functions/methods as well?

@jlaura
Copy link
Member Author

jlaura commented Oct 23, 2014

Agreed on standardization. The lazy way might be to write a decorator for classes that automatically adds a repr method with all of the attributes to a string. It might be better looking and more useful to do this by hand.

@sjsrey
Copy link
Member

sjsrey commented Oct 23, 2014

Let's keep this open to flesh out ideas on a standardized approach.

@jlaura
Copy link
Member Author

jlaura commented Nov 12, 2014

@ipython_tutorial -> repr on geometries would be sweet - not that <pysal.geom_object at .....> is not meaningful. I will start a new PR for the geoms. as a proof of concept.

@ljwolf
Copy link
Member

ljwolf commented Jul 30, 2015

@jlaura is this available in your branches? I was thinking about trying to push on this, maybe also adding special pretty printing for IPython notebook environments.

Github tells me it's from an "unknown repository:"

screen shot 2015-07-30 at 11 54 18 am

@jlaura
Copy link
Member Author

jlaura commented Jul 30, 2015

It looks like I axed the branch, but the commit lives on in an old PR:
c003745

Also checkout how pandas manages the to_* methods. I think that this
approach, and the awesome formatting we see in the statsmodels project give
us a a pretty good roadmap. Thoughts?

https://github.com/pydata/pandas/blob/355b4623d842633746b29b6e7f1724af4cd87dae/pandas/core/format.py
https://github.com/statsmodels/statsmodels/blob/4b55fa4871cf3f1dbd2e30bfe00d80df87d4b340/statsmodels/tsa/vector_ar/output.py#L27
(I think this is the correct class, checkout the make method.

On Thu, Jul 30, 2015 at 11:53 AM, Levi John Wolf notifications@github.com
wrote:

@jlaura https://github.com/jlaura is this available in your branches? I
was thinking about trying to push on this, maybe also adding special pretty
printing for IPython notebook environments.


Reply to this email directly or view it on GitHub
#527 (comment).

@ljwolf
Copy link
Member

ljwolf commented Jul 30, 2015

oh woah, didn't realize these changes got merged in a different PR. Should this get closed?

Yes on the outputting stuff. Taking inspiration from how they design their interfaces is smart, I think. This could go into stuff like an spreg SUMMARY.* call and add to the repr as well as the summaries.

@jlaura
Copy link
Member Author

jlaura commented Jul 30, 2015

I hadn't realized that this was merged either....

What I can do is manually back it out (just that commit)? This must have
have snuck in with another PR due to some sloppy git work on my side.

On Thu, Jul 30, 2015 at 1:06 PM, Levi John Wolf notifications@github.com
wrote:

oh woah, didn't realize these changes got merged in a different PR. Should
this get closed?

Yes on the outputting stuff. Taking inspiration from how they design their
interfaces is smart, I think. This could go into stuff like an spreg
SUMMARY.* call and add to the repr as well as the summaries.


Reply to this email directly or view it on GitHub
#527 (comment).

@ljwolf
Copy link
Member

ljwolf commented Jul 30, 2015

git revert c00374 could be pretty scary, but might work. I can't remember any changes made to Moran that have that commit as a parent.

There's probably a tool to visualize the git graph somewhere. That'd show what would cascade.

@jlaura
Copy link
Member Author

jlaura commented Jul 30, 2015

Since the change in that commit is just the repr addition, I would
likely just manually remove it and then PR that back in. To avoid a nasty
cascade.

Are we better leave it, knowing that someone is looking at adding repr
and other writing items or remove and start fresh?

On Thu, Jul 30, 2015 at 1:43 PM, Levi John Wolf notifications@github.com
wrote:

git revert c00374 could be pretty scary, but might work. I can't remember
any changes made to Moran that have that commit as a parent.

There's probably a tool to visualize the git graph somewhere. That'd show
what would cascade.


Reply to this email directly or view it on GitHub
#527 (comment).

@sjsrey
Copy link
Member

sjsrey commented Jul 30, 2015

I don't think this got merged into master? At least moran.py doesn't look to have the new __repr__

@jlaura
Copy link
Member Author

jlaura commented Jul 30, 2015

+1

https://github.com/pysal/pysal/search?utf8=✓&q=__repr__

No need for a roll back.

On Thu, Jul 30, 2015 at 2:16 PM, Sergio Rey notifications@github.com
wrote:

I don't think this got merged into master? At least moran.py
https://github.com/pysal/pysal/blob/master/pysal/esda/moran.py doesn't
look to have the new repr


Reply to this email directly or view it on GitHub
#527 (comment).

@ljwolf
Copy link
Member

ljwolf commented Aug 3, 2015

I think it got pulled in and out over the course of #601.

That said, this PR is weird and should probably get refreshed since we can't add to it if the source is deleted. Apparently github copies the source into a hidden branch in the target so that the PR can still get merged if the source is deleted.

However, I'm not sure whether you can just reopen the source with the same branch name...

@sjsrey sjsrey mentioned this pull request May 19, 2016
@sjsrey
Copy link
Member

sjsrey commented Jun 10, 2016

Revisit at Scipy 2106 to assign modules to implement this for.

@sjsrey sjsrey closed this Jun 29, 2019
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.

None yet

3 participants