Skip to content

Python code improvements#4

Open
pcurry wants to merge 8 commits intorjbs:mainfrom
pcurry:master
Open

Python code improvements#4
pcurry wants to merge 8 commits intorjbs:mainfrom
pcurry:master

Conversation

@pcurry
Copy link

@pcurry pcurry commented Apr 23, 2013

Hello, I'm Paul Curry.

I was poking at your rx codebase with @danbornside. He has some improvements in mind. The Python coding standard we stick to is PEP8, so we started to clean up the Python code to meet that standard. He started cleaning up the functional code, and I cleaned up the tests. I also added a setup.py, to make this easier to install in Python virtualenv development environments.

So please take a look at the changes I have in this fork. Hopefully either @danbornside will make a pull request soon, or he'll update from my repo and his changes will come in that way.

Thanks!
-Paul Curry

Dan Bornside and others added 4 commits April 16, 2013 17:48
Python types inherit from each other; bool's are numbers and strings are sequences; so they are special cased.
@rjbs
Copy link
Owner

rjbs commented Apr 26, 2013

Hi, Paul. Thanks for your work, here! I'd call my Python rusty, but it was never a very highly refined steel to begin with. I appreciate having you two do a bit of sanity checking. Some of the things I did were clearly sub-optimal.

On the other hand, I am really not thrilled by four space indents. I have no strong objections to most of PEP 8, but that bit is no good for me. If you would redo this work to omit those changes, I would be very happy to apply it. If you'd rather not, I'll probably rework the commits myself in a week or so.

Are you using Rx in Python and finding it useful? That would be delightful to hear. If you're interested in doing even more work on it, you could look at the structured exceptions provided by the Perl implementation and port those over. :-)

Thanks again!

@pcurry
Copy link
Author

pcurry commented Apr 26, 2013

Hey Ricardo,

I've pulled the changes from @danbornside, they are now on my master also. I can look at reindenting the Python this weekend, and maybe doing some other refactoring.

I'm certainly willing to look at the structured exceptions in the Perl implementation, and see how they would translate. Could you please point me to where they are in the Perl implementation? I don't speak Perl, so I'm not really sure where I should be looking and what I should be looking for, to find these exceptions.

@rjbs
Copy link
Owner

rjbs commented Apr 27, 2013

Cool, I look forward to updates from you, thanks for your quick reply!

There are (at least) two places to look at for the structured errors. The spec/schemata files include information about how things should fail, indicating the path to the bad data, the path to the failing check, and the type of error expected. (I apologize: these updates have not been well-documented, and they should be.)

Then maybe look at the code in the Perl test runner which compares the fail it got with the fail it wanted:

https://github.com/rjbs/rx/blob/master/perl/t/lib/Test/RxTester.pm#L162

Checks that do "subchecks" (like checking an entry in a dict against the type in a //rec) use this:

https://github.com/rjbs/rx/blob/master/perl/t/lib/Test/RxTester.pm#L162

Those perform_subchecks end up nesting and, if something deep down throws an Rx exception, it gets repeatedly contextualize-ed on the way up so that the final exception has the whole stack back into it.

I think it sounds more complicated than it is. 😄

Probably much of the work could be ported directly from Perl. You might think you don't speak Perl, but there's not a whole lot of difference between Perl and Python, especially the subsets used in Rx. Anyway, if you end up not wanting to go through with that, I can dig it. But if you do, you will totally have a free beer to claim from me…

@pcurry
Copy link
Author

pcurry commented May 8, 2013

Things got busy for me, so I haven't had the time to put into this. It's still on my radar, fear not.

@rjbs
Copy link
Owner

rjbs commented May 8, 2013

  • Paul Curry notifications@github.com [2013-05-07T20:17:57]

    Things got busy for me, so I haven't had the time to put into this. It's
    still on my radar, fear not.

Great to hear, thanks for the report. Just today I was checking my spam trap
to see whether I'd missed any mail from you. :-) I'm keeping busy myself, so
only feel as rushed as you like.

rjbs

@hunterboerner
Copy link

Still on your radar?

@pcurry
Copy link
Author

pcurry commented Jul 25, 2015

It dropped off my radar, but I have some time. I can look into it.

On Fri, Jul 24, 2015 at 7:22 PM, Theron Boerner notifications@github.com
wrote:

Still on you radar?


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

Updating with 2 years of changes.
@rjbs
Copy link
Owner

rjbs commented Jan 25, 2020

I assume this is long off-the-radar, and am likely to close it. But I have also been away, so I will give it some time to re-surface!

@pcurry
Copy link
Author

pcurry commented Jan 26, 2020

I'll have to pull master and update. And, again, the Python coding standard is 4 spaces per indent level - see PEP 8 https://www.python.org/dev/peps/pep-0008/

@durey
Copy link

durey commented Oct 25, 2020

Hi, what Python library brings TAP.Simple as an import? Is there any using pip?
I've already tried several: pip install TAP/tappy/tap.py without success, I just can't execute from TAP.Simple import *.

@pcurry
Copy link
Author

pcurry commented Oct 25, 2020

Hi, what Python library brings TAP.Simple as an import? Is there any using pip?
I've already tried several: pip install TAP/tappy/tap.py without success, I just can't execute from TAP.Simple import *.

Hi durey! So, what it sounds like you are trying to do is import TAP.Simple.* to use it for testing. I haven't looked at this library in a long time, so I'm not sure how to pull it in. I'll have a look, and see what I can do.

@durey
Copy link

durey commented Oct 25, 2020

Exactly, and thanks! I just wanted to run the tests locally.

@pcurry
Copy link
Author

pcurry commented Oct 27, 2020

@durey It turns out getting this to work is a good question. The Rx Python code is using TAP.Simple, and I'm trying to figure out where it gets TAP.Simple. Tappy doesn't seem to provide it. The official documentation seems to suggest a number of libraries: https://github.com/TestAnything/testanything.github.io/blob/master/producers.md

@rjbs
Copy link
Owner

rjbs commented Oct 27, 2020

It's here: https://github.com/rjbs/tapsimple

I think I wrote it just to get this testing done, and I'm happy to have it ported to anything else. It's not in standard use by … I mean, probably anybody.

@Asday
Copy link

Asday commented Feb 13, 2021

I'd like to point out that PEP8'ing someone else's code is generally destructive and not a great use of one's time and effort.

Here is a really good talk on the topic by Raymond Hettinger.

@durey
Copy link

durey commented Feb 13, 2021

@Asday I haven't watched that 1-hour-long video yet, but in any case, keeping coherence inside a project is by far one of the more important stylistic rules, no matter the specific choices. I am not part of that discussion here, but I see the feedback as a coherence requirement (in this project that is using Pep8) over a Pep8-loving requirement.

@Asday
Copy link

Asday commented Feb 13, 2021

The operative is "someone else's".

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.

5 participants