Skip to content

Description formatting for pre-packaged constraints (example for #195)#196

Draft
cgevans wants to merge 4 commits into
UC-Davis-molecular-computing:devfrom
cgevans:description_format
Draft

Description formatting for pre-packaged constraints (example for #195)#196
cgevans wants to merge 4 commits into
UC-Davis-molecular-computing:devfrom
cgevans:description_format

Conversation

@cgevans
Copy link
Copy Markdown
Collaborator

@cgevans cgevans commented Jul 4, 2022

No description provided.

Copy link
Copy Markdown
Member

@dave-doty dave-doty left a comment

Choose a reason for hiding this comment

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

Ah, I see. But I think this may be sort of surprising behavior, so it might be best to also have a boolean parameter that shuts this off. Although even having the parameter True by default could be confusing: I could imagine someone getting very confused if they try to use curly braces in their string and get some strange error when format is called.

Maybe a good compromise is to put the format in a try/catch:

try:
  description = description.format(
    ...
  )
catch WhateverExceptionHappensWhenFormatGoesWrong:
  print('I tried parsing the description and it failed; to avoid this warning please set the parameter format_description to false, or avoid using curly braces {} in your description')

What do you think?

@cgevans
Copy link
Copy Markdown
Collaborator Author

cgevans commented Jul 4, 2022

That could make sense. Here's an attempt at it for this one case: the formatting should give either a KeyError (keyword not found, in which case the user may well have been trying to use the formatting) or ValueError (for parsing).

@dave-doty dave-doty changed the base branch from main to dev July 5, 2022 21:30
@dave-doty
Copy link
Copy Markdown
Member

Do you want to leave this PR as a draft, or is it ready to merge?

@dave-doty
Copy link
Copy Markdown
Member

Actually, some things should probably change, for example adding this feature to other functions that create constraints.

@cgevans
Copy link
Copy Markdown
Collaborator Author

cgevans commented Jul 7, 2022

Yes, this isn't done yet, I need to do the same thing elsewhere, and add some tests.

@dave-doty
Copy link
Copy Markdown
Member

Yes, this isn't done yet, I need to do the same thing elsewhere, and add some tests.

Maybe merge the current dev into this branch. I've just added two new functions (described in #191 (comment)) that would also have their description fields altered. (Or in this case, descriptions)

@cgevans cgevans force-pushed the description_format branch from da86e7a to 2d1a64d Compare July 20, 2022 23:56
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