soachishti/moss.py

Modify UploadFile

YsingYang opened this issue ยท 11 comments

YsingYang commented

Hello
I perfer to using filename_fixed = file.replace(" ", "_") instead of filename_fixed = os.path.basename(file).replace(" ", "_").

In my case, I have many folders in submission folder, but there is file which have same name(MainActivity.java) in different folder. So the result report can not tell me which folder it is in.More importantly, in report, the different link MainActivity.java point to the same MainActivity.java!

Directory Structure Example

                      - No.1 - MainActivity.java
                      |
Submission          - - No.2 - MainActivity.java
                      |
                     -  No.3- MainActivity.java

If you agree, I would create my pull request :)

nareddyt commented

I agree this is a problem, I had to create a similar workaround to get this working with our autograder.

For long term use, I believe the better implementation would be to add a new parameter to addFiles(...) that takes in a file_path. This way, users can add files either by file_name or file_path.

Thoughts on this?

YsingYang commented

Do You mean add a new parameter to addFilesByWildcard?

nareddyt commented

I believe you would do it for addFiles(...), addFilesByWildcard(...), and addBaseFiles(...)

YsingYang commented

Maybe you are right,

soachishti commented

Yeah! that is an problem. In our we were renaming files to the "Roll no" because it make the Moss report simpler for us to review (filename are students roll no eg. "[assignment no]-[roll no].py")...

I agree with the @nareddyt solution.

@YsingYang Thanks for pointing it out, let have a pull request for this.

soachishti commented

Adding parameters in addFiles(...) and etc, will break the logic of uploadFile(...); it must have actual file path to read the file to upload... what if we take the parameter (naming can be file_path, file_name) in constructor and use it within uploadFile(...) with the code, @YsingYang shared in initial message.

@nareddyt @YsingYang What do you say?

nareddyt commented

Yes, having a file_path and a display_name makes a lot more sense! Then you would pass in the full path to the file, but also a friendly display name.

@YsingYang do you want to handle that or should I do that?

  • ๐Ÿ‘2
soachishti commented

Got it =]

YsingYang commented

@nareddyt Sorry, I'm late.
I am not familiar with python and this project(ๅ›ง). So, if you can handle you can do that, I will try my best to help(some suggestion and test). OK?

  • ๐Ÿ‘1
  • ๐ŸŽ‰1
YsingYang commented

@soachishti I think it is not necessary to create my pull request :), and modify the project by @nareddyt idea.
Do you think so?

nareddyt commented

I'll take care of it! I'll have the pull request done by this weekend, pretty busy till then

  • ๐Ÿ‘1