Added command to export reviewer's votes #650

the-xperimentalist · 2020-03-22T08:05:36Z

Fix for: #553

coveralls · 2020-03-22T08:07:37Z

Coverage decreased (-0.05%) to 67.308% when pulling b3470b5 on prasoonmayank:master into b8309fb on pythonindia:master.

junction/proposals/management/commands/export_conf_proposals.py

palnabarun · 2020-03-22T08:11:22Z

@prasoonmayank Left some comments.

Also, please lint your code before committing. You can do so by running nox -s lint

sks444

Hi @prasoonmayank Can you run the command you created locally to check if it is creating the required csv file and storing data into it? In doing so you can find these errors I mentioned.

junction/proposals/management/commands/export_conf_proposals.py

requirements.txt

the-xperimentalist · 2020-03-24T20:58:33Z

Will be completing proper testing with corner cases by today

sks444 · 2020-03-25T05:14:18Z

@prasoonmayank, It looks like you have committed your env file?

sks444

Nice progress @prasoonmayank. I have left some comments have a look.

Also, I think it would be good to have different methods to do all the logic and let the command handler call those methods, instead of directly putting all the logic in the handler.

P.S. Try always to use a new development branch(other than master) to create a pull request. Have a look at our contributing guide. :)

junction/proposals/management/commands/export_conf_proposals.py

ananyo2012 · 2020-04-04T15:30:23Z

@prasoonmayank Can you rebase your changes and run the linter again. There is a recent PR that has been merged with a change in the linting approach.

the-xperimentalist · 2020-04-04T18:28:30Z

Hey @ananyo2012 is there a way to run the lint code without nox? Since, earlier, nox was calling a flake8 command, and was able to lint it. Since, for some reason, nox is not working in Mac

pradyunsg · 2020-04-04T20:20:48Z

@prasoonmayank Why is nox not working for you? Could you share the error message?

pradyunsg · 2020-04-04T21:08:51Z

Also, yes, you can run the linters without nox, by running pre-commit directly (pip install pre-commit and then pre-commit run --all-files).

ananyo2012 · 2020-04-05T04:05:10Z

I will try to run the latest linting changes locally. We also got a report earlier regarding issues running test locally.

…

On Sun, Apr 5, 2020 at 2:39 AM Pradyun Gedam ***@***.***> wrote: Also, yes, you can run the linters without nox, by running pre-commit directly (pip install pre-commit and then pre-commit run --all-files). — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

the-xperimentalist · 2020-04-05T07:27:50Z

@prasoonmayank Why is nox not working for you? Could you share the error message?

There were essentially two problems in my case:

pipx wasn't installing nox.
For some reason, nox command in Mac was setup in a different folder, since there are multiple python versions installed probably?
Regarding linting, with the latest changes, we are able to lint with the pre-commit command only with the python version being 3.6

ananyo2012 · 2020-04-05T08:40:41Z

@prasoonmayank There is an issue reported for Mac systems in #660 . What python versions do you have in your system ? Instead of using nox can you try running tests using pytest --cov -v --tb=native.

pre-commit should be able to lint in all versions. Are you seeing any failures related to isort ? That's a known issue introduced recently. Except that other linting tests should work.

the-xperimentalist · 2020-04-05T19:09:56Z

#660

I installed python3.6 for the pre-commit linter to work.
Regarding the test coverage. All the test cases have been passing, but the build is failing for some reason. Can you suggest some probable reason for the same?

pradyunsg · 2020-04-05T20:10:04Z

@prasoonmayank If you click details on the failing check, you can see the CI runs, and then clicking on the specific job reveals the output of that job.

In this case, the linters are failing for you.

ananyo2012 · 2020-04-06T03:47:37Z

The isort fails are a valid one I mentioned in #663 cc: @pradyunsg

pradyunsg · 2020-04-06T17:01:00Z

Hey @prasoonmayank, could you file an issue describing why you aren't able to install nox (including the error messages if you see any)? If it's not working for a contributor, that's a bug in either our developer tools, or developer documentation. :)

pradyunsg · 2020-04-06T17:02:36Z

Other than that, please feel free to go ahead and merge master into this PR / rebase this PR on master.

sayanchowdhury · 2020-04-17T06:29:28Z

Before merging please make sure the commits are squashed.

the-xperimentalist · 2020-04-19T13:54:50Z

Hey @palnabarun Please review the pr. As it is in requested changes state right now

palnabarun

@prasoonmayank Left some comments.

Other than those, I think this should be good to go.

palnabarun · 2020-04-19T15:43:49Z

junction/proposals/management/commands/export_conf_proposals.py

+            user = User.objects.get(id=options.get("user_id"))
+        except User.DoesNotExist:
+            self.stdout.write("Invalid user")
+            return


Exception cases should be returned with a Non-Zero Exit Code.

handled. though, just curious, what is the advantage?

@palnabarun have made the changes, please rereview

palnabarun · 2020-04-19T15:43:57Z

junction/proposals/management/commands/export_conf_proposals.py

+            return
+        except Conference.DoesNotExist:
+            self.stdout.write("Invalid conference")
+            return


Same as above.

palnabarun · 2020-04-19T15:44:08Z

junction/proposals/management/commands/export_conf_proposals.py

+
+        if not is_conference_moderator(user=user, conference=conference):
+            self.stdout.write("The user id is not a conference moderator")
+            return


Same as above.

ananyo2012 · 2020-05-04T15:14:11Z

junction/proposals/management/commands/export_conf_proposals.py

+            help="Enter the conference slug where to export reviewer votes from",
+        )
+
+        parser.add_argument("--user_id", default=None, help="Enter your user id")


What is the need of userid ? The command is supposed to be used by the admin from the console so the admin must be logged in to the application server.

Not sure, but there are multiple admin users, right?

Yes whoever has login access can use this command. No need to check for user id. Usually whoever is managing junction is also the one managing the server. Also user id's are not user friendly

ananyo2012 · 2020-05-04T15:17:39Z

junction/proposals/management/commands/export_conf_proposals.py

+        parser.add_argument(
+            "--conference_slug",
+            default=None,
+            help="Enter the conference slug where to export reviewer votes from",


Update the help text to Conference slug

ananyo2012 · 2020-05-04T15:32:43Z

junction/proposals/management/commands/export_conf_proposals.py

+        )
+        vote_values_list = [v.vote_value for v in proposal_vote_values]
+        vote_values_desc = tuple(i.description for i in proposal_vote_values)
+        header = (


Why is there an extra pair of brackets ?

ananyo2012 · 2020-05-04T15:57:03Z

junction/proposals/management/commands/export_conf_proposals.py

+                    ]
+                )
+                row = {
+                    header[0]: p.proposal_type.name,


I think instead of csv.DictWriter you should simply use csv.writer and use lists to write each row. This reduces this complexity of creating a dictionary in each iteration and makes the code simpler. e.g.

junction/junction/proposals/dashboard.py

Line 326 in e8cefd7

row = (p.proposal_type.name, p.title, p.get_reviewer_votes_sum(),

Using dictwriter would be a lot more accurate, right? Always ensuring which values go for which fields? Probably, never creating confusion for future devs on which key belongs to which key?

Please rename the vote_details variable to reviewer_votes_count in that case

ananyo2012 · 2020-06-07T20:45:47Z

junction/proposals/management/commands/export_conf_proposals.py

+
+                csv_contents.append(row)
+
+        csv_file_name = "%s-%s.csv" % (user.username, conference.name)


There is no section name in the filename. This will overwrite the file each time as it is opened in w mode.

aniruddha2000 · 2020-08-03T15:41:25Z

@mpras97 are you still working on this?

the-xperimentalist · 2020-08-06T08:20:20Z

@mpras97 are you still working on this?

Yes. Willl resolve these comments

the-xperimentalist added 3 commits March 22, 2020 13:31

Python 3.5 type strings

a383ac2

Added command to export as excel file

b4ddc51

Merge branch 'master' of https://github.com/prasoonmayank/junction

300ed87

palnabarun requested changes Mar 22, 2020

View reviewed changes

junction/proposals/management/commands/export_conf_proposals.py Outdated Show resolved Hide resolved

the-xperimentalist added 3 commits March 22, 2020 23:48

Change export to csv

3822dd3

Remove unused import

75dce2e

Message text change

bc5a473

sks444 suggested changes Mar 22, 2020

View reviewed changes

junction/proposals/management/commands/export_conf_proposals.py Outdated Show resolved Hide resolved

junction/proposals/management/commands/export_conf_proposals.py Outdated Show resolved Hide resolved

palnabarun requested changes Mar 23, 2020

View reviewed changes

requirements.txt Outdated Show resolved Hide resolved

the-xperimentalist added 3 commits March 25, 2020 01:41

Working command

2280969

Converted from usage to csv

b44eb7a

Remove pandas from requirements

4e48be2

Remove env file

664f133

sks444 suggested changes Mar 25, 2020

View reviewed changes

junction/proposals/management/commands/export_conf_proposals.py Outdated Show resolved Hide resolved

junction/proposals/management/commands/export_conf_proposals.py Show resolved Hide resolved

junction/proposals/management/commands/export_conf_proposals.py Outdated Show resolved Hide resolved

ananyo2012 closed this Mar 26, 2020

ananyo2012 reopened this Mar 26, 2020

Resolved comments and run linter

5ad9437

the-xperimentalist added 2 commits April 4, 2020 23:30

Removed return

e7f2495

Merge branch 'master' of https://github.com/pythonindia/junction

dc85222

pradyunsg changed the title ~~Added command~~ Added command to export reviewer's votes Apr 5, 2020

Lint code based upon latest changes

fc3cb1c

pradyunsg added the type/enhancement label Apr 6, 2020

Merge branch 'master' into master

9d87dba

the-xperimentalist added 2 commits April 19, 2020 19:23

Fix lint issues

ba50508

Merge branch 'master' of https://github.com/prasoonmayank/junction

625637c

palnabarun requested changes Apr 19, 2020

View reviewed changes

Resolved comments

99076ae

the-xperimentalist requested a review from palnabarun April 27, 2020 17:44

the-xperimentalist added 2 commits May 2, 2020 20:51

Merge branch 'master' into master

b3470b5

Merge branch 'master' into master

79c0fbe

the-xperimentalist requested a review from ananyo2012 May 4, 2020 05:44

ananyo2012 reviewed May 4, 2020

View reviewed changes

the-xperimentalist and others added 2 commits May 30, 2020 10:45

Merge branch 'master' into master

c8790fb

Resolved some comments

3bcc98f

ananyo2012 reviewed Jun 7, 2020

View reviewed changes

palnabarun mentioned this pull request Jul 30, 2020

Create a management code to export reviewers vote #553

Open

Merge branch 'master' into master

13bf0c1


		csv_contents.append(row)

		csv_file_name = "%s-%s.csv" % (user.username, conference.name)

Added command to export reviewer's votes #650

Are you sure you want to change the base?

Added command to export reviewer's votes #650

Conversation

the-xperimentalist commented Mar 22, 2020

coveralls commented Mar 22, 2020 • edited Loading

palnabarun commented Mar 22, 2020

sks444 left a comment • edited Loading

Choose a reason for hiding this comment

the-xperimentalist commented Mar 24, 2020

sks444 commented Mar 25, 2020

sks444 left a comment

Choose a reason for hiding this comment

ananyo2012 commented Apr 4, 2020

the-xperimentalist commented Apr 4, 2020

pradyunsg commented Apr 4, 2020

pradyunsg commented Apr 4, 2020

ananyo2012 commented Apr 5, 2020 via email

the-xperimentalist commented Apr 5, 2020

ananyo2012 commented Apr 5, 2020

the-xperimentalist commented Apr 5, 2020

pradyunsg commented Apr 5, 2020

ananyo2012 commented Apr 6, 2020

pradyunsg commented Apr 6, 2020 • edited Loading

pradyunsg commented Apr 6, 2020

sayanchowdhury commented Apr 17, 2020

the-xperimentalist commented Apr 19, 2020

palnabarun left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aniruddha2000 commented Aug 3, 2020

the-xperimentalist commented Aug 6, 2020

coveralls commented Mar 22, 2020 •

edited

Loading

sks444 left a comment •

edited

Loading

pradyunsg commented Apr 6, 2020 •

edited

Loading