Skip to content

Remove replace_ argument for files_filtered again#580

Merged
mwcraig merged 1 commit intoastropy:masterfrom
MSeifert04:remove_replace_
Oct 31, 2017
Merged

Remove replace_ argument for files_filtered again#580
mwcraig merged 1 commit intoastropy:masterfrom
MSeifert04:remove_replace_

Conversation

@MSeifert04
Copy link
Contributor

@MSeifert04 MSeifert04 commented Oct 24, 2017

The replace_ argument was basically a good idea but as #559 and #557 show whitespaces are not the only problematic character. - is quite frequently used and sometimes even .. Other invalid characters for keyword arguments are quite rare.

However it might be better to just document the workaround (passing in an unpacked dictionary) than actually inventing a functionality that tries to cover these cases in an easy to use way.

If we make replace_ a dictionary for characters to translate that will be very awkward to use (because you could end up inventing different placeholders for different characters) and when you're passing in a dictionary already one might as well pass in the dictionary containing the key-value pairs.

Note that replace_ hasn't been released yet so this doesn't need a deprecation.

@MSeifert04 MSeifert04 added this to the 1.3 milestone Oct 24, 2017
@MSeifert04
Copy link
Contributor Author

MSeifert04 commented Oct 24, 2017

xref: #481 #539

@mwcraig
Copy link
Member

mwcraig commented Oct 30, 2017

Any thoughts on this, @bsipocz?

@mwcraig
Copy link
Member

mwcraig commented Oct 31, 2017

Ideally we'd talk about this in the narrative documentation, but I think those need a much larger overhaul that makes sense as part of ccdproc 2.0, so I'm merging this as-is.

@mwcraig mwcraig merged commit 5d8f257 into astropy:master Oct 31, 2017
@MSeifert04 MSeifert04 deleted the remove_replace_ branch October 31, 2017 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants