Skip to content

Add filter logic to export_records#85

Merged
sburns merged 5 commits intoredcap-tools:masterfrom
erikh360:add-filter-logic-to-export-records
Oct 12, 2018
Merged

Add filter logic to export_records#85
sburns merged 5 commits intoredcap-tools:masterfrom
erikh360:add-filter-logic-to-export-records

Conversation

@erikh360
Copy link
Copy Markdown
Contributor

This adds a filter_logic argument to the export_records function on the Project object.

Unfortunately I don't have access to a test project/token to update the tests.

@sburns
Copy link
Copy Markdown
Collaborator

sburns commented Sep 21, 2018

Thanks for your PR! I think this is your first on PyCap?

Unfortunately I don't have access to a test project/token to update the tests.

me neither 😢 . I could see improving our tests to do one of two things:

  1. Find another institution where we can host an example project. Still brittle because it's going to be owned by someone, they'll eventually leave, etc.
  2. Rewrite our tests to not need the internet.

Would you like to do either one?

@erikh360
Copy link
Copy Markdown
Contributor Author

Thanks for your reply @sburns. Yes my first one!

Unfortunately, I don't have direct access to a instance that can host a example project. How much work would it be to set one up?

Time is also not really on my side to rewrite all the tests. I also haven't checked how much work this would be.

@erikh360
Copy link
Copy Markdown
Contributor Author

erikh360 commented Oct 4, 2018

@sburns I still need to sort out some things, but what do you think of the solution?

@erikh360
Copy link
Copy Markdown
Contributor Author

erikh360 commented Oct 5, 2018

@sburns Green! but yeah, let me know what you think of the solution.

Copy link
Copy Markdown
Collaborator

@sburns sburns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome! Thank you so much for working on this, I really like the approach you took and TIL about the responses project. I've used betamax in the past but this seems very effective.

Comment thread test/test_project.py Outdated
with self.assertRaises(RedcapError):
self.reg_proj.export_file(record, field)

# self.reg_proj.delete_file(record, field)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this related to the tests not actually making any kind of side effect on the real server anymore?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, as everything is mocked now.

Comment thread test/test_project.py Outdated
# grrr coerce implicilty converted floats to str(int())
for col in ['matrix1', 'matrix2', 'matrix3', 'sex']:
df[col] = map(lambda x: str(int(x)) if pd.notnull(x) else '', df[col])
# for col in ['matrix1', 'matrix2', 'matrix3', 'sex']:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's better just to remove old code than comment it out, that's why we have version control :)

Comment thread test/test_project.py Outdated
return [{'study_id': '1',
'dob': date_string}]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's white space in your new lines, can you remove that?

@erikh360
Copy link
Copy Markdown
Contributor Author

erikh360 commented Oct 8, 2018

@sburns awesome! I'll do some cleanup and commit soon!!

responses is an awesome package and it's usually possible to do it much cleaner, but everything in redcap goes over the same api endpoint so it's a bit clunky.

@erikh360
Copy link
Copy Markdown
Contributor Author

@sburns Hi, reminder about this 🙂

@sburns sburns merged commit d9d3dea into redcap-tools:master Oct 12, 2018
@erikh360
Copy link
Copy Markdown
Contributor Author

Hi @sburns Thanks for the merge!

Can you please create a new version? 🙂

@erikh360
Copy link
Copy Markdown
Contributor Author

@sburns Reminder to please create a release. 🙂

@sburns
Copy link
Copy Markdown
Collaborator

sburns commented Oct 17, 2018

Actually in the mean time, @erikh360 do you know how to install from github?

You should be able to do pip install -e git+https://github.com/redcap-tools/PyCap.git#egg=PyCap

@erikh360
Copy link
Copy Markdown
Contributor Author

@sburns ah yes thanks, that was my backup plan.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants