load_profiles should work with PurePath types

[gwaybio]

Description

Fixing the issue @ErinWeisbart described in #284 - the issue is within the load_profiles() function and a type check that fails to account for pathlib.PurePath() objects.

What is the nature of your change?

• Bug fix (fixes an issue).
• Enhancement (adds functionality).
• Breaking change (fix or feature that would cause existing functionality to not work as expected).
• This change requires a documentation update.

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.
• I have deleted all non-relevant text in this pull request template.

[gwaybio]

I've added a test https://github.com/cytomining/pycytominer/actions/runs/5149178757/jobs/9271863384?pr=285#step:5:118 which fails with the current implementation

[codecov-commenter]

Codecov Report

Merging #285 (3938ac7) into master (6bef01b) will increase coverage by 0.11%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #285      +/-   ##
==========================================
+ Coverage   95.00%   95.12%   +0.11%     
==========================================
  Files          57       57              
  Lines        3046     3057      +11     
==========================================
+ Hits         2894     2908      +14     
+ Misses        152      149       -3     

Flag	Coverage Δ
unittests	95.12% <100.00%> (+0.11%)	⬆️

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

Impacted Files	Coverage Δ
pycytominer/cyto_utils/load.py	90.90% <100.00%> (+3.56%)	⬆️
pycytominer/tests/test_cyto_utils/test_load.py	100.00% <100.00%> (ø)

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

[d33bs]

Nice work! Overall I felt good about the changes here and left a few comments + suggestions on things that might warrant attention. Please don't hesitate to let me know if you have any questions.

[gwaybio]

it also seems that my black formatting may have introduced some unintended line changes. Is there a way for me to avoid these sorts of updates? I thought pre-commit solved this?

[d33bs]

Nice changes! Thanks for replying to the comments and for making updates. I left a few more suggestions with this review.

On the question about Black / pre-commit formatting: I did notice the formatting changes and when I ran black --check on modified Python files it seemed happy with things. To a certain extent, this might be a lot of what we start to see in terms of gradual integration of Black throughout (thinking of incoming changes with #279).

To avoid changes to certain lines we could add # fmt: off above the lines in question (reference). I also wonder if adding additional configuration options specific to pycytominer's opinion about formatting could help (reference, specifically the line-length option may clarify things if we chose to modify this from the default). Using a file-based configuration for Black through a TOML file might also follow or prepare us for changes in #278 .

Separately, I wonder too if a future PR which only applies Black to all files could help with avoiding confusion mid-PR for lines which are formatted differently (theoretically this would be a style-only change, meaning no testing / operational concerns should appear).

[gwaybio]

Thanks for the re-review! I will merge after tests pass:

To avoid changes to certain lines we could add # fmt: off above the lines in question (reference). I also wonder if adding additional configuration options specific to pycytominer's opinion about formatting could help (reference, specifically the line-length option may clarify things if we chose to modify this from the default). Using a file-based configuration for Black through a TOML file might also follow or prepare us for changes in #278 .

I think this would be a good idea, although i am totally fine with having default opinions, which will reduce maintenance burden.

Separately, I wonder too if a future PR which only applies Black to all files could help with avoiding confusion mid-PR for lines which are formatted differently (theoretically this would be a style-only change, meaning no testing / operational concerns should appear).

This sounds like a great idea! Maybe something for Dayna to contribute? :)