Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Move get_full_customization_args from exp_domain to utils #4968

Closed
pranavsid98 opened this issue May 22, 2018 · 15 comments
Closed

Move get_full_customization_args from exp_domain to utils #4968

pranavsid98 opened this issue May 22, 2018 · 15 comments
Labels
backend code-health

Comments

@pranavsid98
Copy link
Contributor

pranavsid98 commented May 22, 2018 •
edited by seanlip
Loading

Related to #4946 -- see #4946 (comment)

The get_full_customization_args method currently in exp_domain is not relevant there anymore. Now, it is being used from stats_domain as well, and it is better to move this method to utils.py instead.
@seanlip
Copy link
Member

seanlip commented May 23, 2018 •
edited
Loading

Hi @pranavsid98 -- this issue description is insufficient. Please write a short description explaining the context and what needs to be done. Thanks!

@jacobdavis11
Copy link
Member

@pranavsid98 are you currently working on this?

@pranavsid98
Copy link
Contributor Author

@jacobdavis11 Its not a blocking issue and I thought Id get to this after my GSOC. Hope thats fine. Thanks.

@jacobdavis11
Copy link
Member

@pranavsid98 any updates on this?

@anmolshkl
Copy link
Contributor

@pranavsid98 GSoC is over :)
Would you be able to pick this up now?

This was referenced Sep 21, 2018
Merged
Merged
@seanlip
Copy link
Member

seanlip commented Sep 23, 2018

Checked with @pranavsid98, he'll take this up and complete it by 30 Sep. Thanks!

@seanlip seanlip assigned brianrodri and unassigned pranavsid98 Oct 4, 2018
@tjiang11
Copy link
Contributor

What's the status of this issue? @brianrodri

@nithusha21
Copy link
Contributor

Hi @brianrodri, any updates?

@brianrodri
Copy link
Contributor

I don't have much bandwidth to work on this, so I'm unassigning myself and giving it a space in the project page for someone to pick up.

@brianrodri brianrodri removed their assignment Oct 29, 2018
@ankita240796
Copy link
Contributor

Hi @brianrodri, May I take this up, Thanks!

@brianrodri
Copy link
Contributor

Sure, it's all yours @ankita240796! Thanks!

@ankita240796
Copy link
Contributor

Thanks! @brianrodri!

@ankita240796
Copy link
Contributor

ankita240796 commented Dec 20, 2018 •
edited
Loading

Hi @brianrodri, get_full_customization_args is currently present in state_domain.py after this PR. Also if we are moving this function to utils.py, I feel the same should be done for validation as well(validate_customization_args_and_values defined in state_domain.py as well). What do you suggest?

EDIT: I have issued a PR moving both the functions. If that does not look fine, I will move back validation function to state_domain.py

@ankita240796 ankita240796 mentioned this issue Dec 20, 2018
Merged
@brianrodri
Copy link
Contributor

Thanks @ankita240796, I agree that moving them both makes sense! I do think, however, that utils is too general for them. Let's move them to schema_util instead?

@ankita240796
Copy link
Contributor

Yes, moving them to schema_util sounds good. I have made the changes in the PR.

brianrodri pushed a commit that referenced this issue Dec 21, 2018
@ankita240796 @brianrodri
Fix #4968: Move customization args method from state_domain to custom…
a5aaaf6 
…ization_args_util (#6015)

* Remove customization arg methods from state and stats domain

* Add customization arg methods to utils

* Add test

* Fix minor nit

* Update test for checking customization arg after validation

* Fix circular dependency

* Move functions to schema_utils to break circular dependency

* Improve variable naming

* Create customization arg utils
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
backend code-health
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
10 participants
@brianrodri @kevinlee12 @anmolshkl @seanlip @tjiang11 @nithusha21 @jacobdavis11 @ankita240796 @pranavsid98 @DubeySandeep