cell_locations: Writing output to S3 was failing; fixed this

[shntnu]

modified from EmbeddedArtistry

Description

The code failed when the output was on S3; I needed to fix a bug that checks for the file's existence on S3.

What is the nature of your change?

• Bug fix (fixes an issue).

Checklist

Please ensure that all boxes are checked before indicating that a pull request is ready for review.

• I have read the CONTRIBUTING.md guidelines.
• My code follows the style guidelines of this project.
• I have performed a self-review of my own code.
• I have commented my code, particularly in hard-to-understand areas.
• I have made corresponding changes to the documentation.
• My changes generate no new warnings.
• New and existing unit tests pass locally with my changes.
• I have added tests that prove my fix is effective or that my feature works. No; See Increase test coverage of cell_locations by figuring out how to use moto #265
• I have deleted all non-relevant text in this pull request template.

[codecov-commenter]

Codecov Report

Merging #271 (02f319e) into master (a5ae6c8) will not change coverage.
The diff coverage is 0.00%.

@@           Coverage Diff           @@
##           master     #271   +/-   ##
=======================================
  Coverage   95.07%   95.07%           
=======================================
  Files          57       57           
  Lines        3044     3044           
=======================================
  Hits         2894     2894           
  Misses        150      150           

Flag	Coverage Δ
unittests	95.07% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pycytominer/cyto_utils/cell_locations.py	84.21% <0.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[shntnu]

@d33bs please note that I could not add a test to check for this; see #265

[d33bs]

Thank you for this contribution @shntnu ! I'd like to request changes on this PR mostly with concerns to exception handling and general discussion.

Some additional items besides those mentioned in the comments:

• I'd ask that you please update the PR description with a root cause or need for the PR. It looks like the description still include the template content. If appropriate, please also update the PR title to the focus area as well. Doing this will help record what changes are occurring and why.
• Given that this occurred partially because of a lack of unit/functional tests, would it make sense to write a quick test to make sure we cover this scenario? Even a small test might provide enough coverage to incrementally begin addressing any unforeseen gaps.

What do you think? Interested in your thoughts - thank you in advance!

[shntnu]

• I'd ask that you please update the PR description with a root cause or need for the PR. It looks like the description still include the template content. If appropriate, please also update the PR title to the focus area as well. Doing this will help record what changes are occurring and why.

Done

• Given that this occurred partially because of a lack of unit/functional tests, would it make sense to write a quick test to make sure we cover this scenario? Even a small test might provide enough coverage to incrementally begin addressing any unforeseen gaps.

Unfortunately, I don't have the expertise to write such a test because it involves the use of moto (#265)

I did create a stub for this here (see Details) #257 (comment) but I didn't have the expertise to push this further.

[d33bs]

Thanks @shntnu and apologies for my delay, this LGTM!

I do think we should prioritize testing to make sure work like this is covered by some form of testing in the future by addressing the issue you created. Just the same, I understand the need to move forward to balance progress + testing assurance.

@gwaybio - do you have any thoughts here as well?

[gwaybio]

LGTM! @shntnu - ok to merge?

[gwaybio]

actually, @shntnu - feel free to merge when you're happy (I assume you have write access to this repo).

[gwaybio]

@shntnu - doing some pycytominer cleanup. Is this good to merge?

[shntnu]

Oops, sorry. All set!