Add OS module

[glaukiol1]

OS Module

I added some of the basic functions/globals of the OS module, these include;

• Functions
  • os.getcwd()
  • os.getcwdb()
  • os.chdir()
  • os.getenv()
  • os.getpid()
  • os.putenv()
  • os.unsetenv()
  • os._exit()
  • os.system()
• Globals
  • os.error
  • os.environ

I also added a py.Println function, in which you can find the definition for in py/utils.py. It looks something like this

func Println(self Object, args ...string) (int,error)

This is my first contribution to gpython, so please give me feedback and other things I should add.
The first two commits were tests i made to figure out how to create a module; I was going to make a JSON module, but then I saw that gpython didnt have an OS module, and I thought that was more important than a JSON module at this time.
I've also included os.test.py, a file which you can run with gpython to check if the OS module is working properly.

Thanks!

[raff]

Looks pretty good to me, but I would clean it up and make it more idomatic (I have added some notes).

Also, if possible the errors should be as close as possible to the error reported by CPython (I think the types are correct but the messages are different, unless you did it right but the messages have changed between Python 3.4 and Python 3.9, that is what I tried).

[glaukiol1]

Hi,
I have added more commits resolving most of your comments. I have two questions;

• How can i get access to gpythons stdout?
• Is there a place where I can correspond messages & error types?

Thanks!

@raff

[raff]

Thanks for the updates.

Regarding stdin, see how I did it for the "print" builtin:
https://github.com/go-python/gpython/blob/master/builtin/builtin.go#L192-L196 and https://github.com/go-python/gpython/blob/master/builtin/builtin.go#L205-L219

Maybe it would be worth extracting a method that can be used to print to stdout, or you can py.Call builtins.print

[raff]

Regarding the error messages as far as I know there isn't an easy way to get the messages from CPython, the best way may be to write a test that you can run through both Cpython and gpython and compare (I think that's what the pytest stuff is supposed to do, with the files in the "tests" folder for each module.

[glaukiol1]

Hi,
I think i fixed everything.
I added a Println function in py/util.py, it prints to the standard gpython output. Anything else i need to do? When is this PR going to be merged?

@raff

Thanks

[codecov]

Codecov Report

Merging #169 (c4d5861) into master (7e10deb) will decrease coverage by 1.07%.
The diff coverage is 28.75%.

❗ Current head c4d5861 differs from pull request most recent head 2c8ecee. Consider uploading reports for the commit 2c8ecee to get more accurate results

@@            Coverage Diff             @@
##           master     #169      +/-   ##
==========================================
- Coverage   73.71%   72.64%   -1.08%     
==========================================
  Files          73       73              
  Lines       11982    11920      -62     
==========================================
- Hits         8833     8659     -174     
- Misses       2545     2672     +127     
+ Partials      604      589      -15     

Impacted Files	Coverage Δ
py/util.go	0.00% <0.00%> (ø)
stdlib/stdlib.go	54.88% <ø> (ø)
stdlib/os/os.go	32.83% <32.83%> (ø)
py/bytes.go	28.07% <0.00%> (-17.10%)	⬇️
py/args.go	73.00% <0.00%> (-16.73%)	⬇️
py/complex.go	31.74% <0.00%> (-0.78%)	⬇️
py/type.go	51.83% <0.00%> (-0.39%)	⬇️
parser/y.go	94.90% <0.00%> (-0.33%)	⬇️
stdlib/math/math.go	79.03% <0.00%> (-0.29%)	⬇️
py/import.go	15.62% <0.00%> (-0.10%)	⬇️
... and 12 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7e10deb...2c8ecee. Read the comment docs.

[sbinet]

hi,

thanks for the PR.
please find below a first run of comments and suggestions.

(I'll try to come back at it in the coming days, but I am a bit under the weather this days...)

[glaukiol1]

Hi @sbinet,

Thanks for your review. I have resolved all of them, but the last one, specifically this one,

perhaps, instead of self Object, directly require self to be *Module, or the correct Context? or, to follow Go's lead, require a "file-like" object? (but admittedly, that function should be called Fprintln...)

I think that for simplicity for calling it in embedded module functions, it is very simple to pass the self py.Object argument than to get a *Module or Context.

Any other things I need to implement? And when will this PR be ready to merge? It is getting quite long... 54 commits.

Also, after i'm done with this, what other task can I take up? Im looking to contribute more.

Thanks!

[sbinet]

actually, this whole loop could be re-written as:

line := strings.Join(args, " ") + "\n"
_, err := Call(write, Tuple{String(line)}, nil)

?
I must admit I don't completely understand the strings.Contains(v, "\n") dance above :}

[glaukiol1]

Hello,

Thanks for your review. I have resolved all but the last one, specifically:

actually, this whole loop could be re-written as:

line := strings.Join(args, " ") + "\n"
_, err := Call(write, Tuple{String(line)}, nil)

...

About this:

I must admit I don't completely understand the strings.Contains(v, "\n") dance above :}

It is because if we add a space between each argument, and if the argument has a newline, the newline will start with a space. An example would be:

# CPython
>>> print("hello\n","second line")
# output
hello
second line

You would see that the second line does not start with a space. However if we implement this: line := strings.Join(args, " ") + "\n", the output will look like this:

# ...
hello
 second line # the line will start with a space.

Thanks!

[sbinet]

ok.
but then I am not sure which version of CPython the proposed behaviour is mimicking.
on my ArchLinux machine that runs CPython-3.10.2 and in a Docker container running CPython-3.4.10:

$> docker run --rm -it python:3.4
Python 3.4.10 (default, Mar 20 2019, 00:50:15) 
[GCC 6.3.0 20170516] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> print("hello\n","world")
hello
 world

with which OS/CPython-version are you seeing this behaviour?

[glaukiol1]

Hello,
I'm running Python 3.9.8 on darwin.

@sbinet

[glaukiol1]

Although I do see that in every other enviroment it works like you said, so i suggest we just implement line := strings.Join(args, " ") + "\n".

[glaukiol1]

@sbinet Thanks for your ping, I resolved the merge conflict.

[sbinet]

LGTM.

could you send yet another PR against the go-python/license, adding yourself to the AUTHORS and/or CONTRIBUTORS files?

then I'll merge this PR in.

thanks a lot for sticking with us.

[sbinet]

(don't worry about the merge conflicts. I'll handle them)

[drew-512]

Love it! Awesome work, gents!

[sbinet]

ping?

[sbinet]

pong?

[sbinet]

needs go-python/license#18

[sbinet]

done :)