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.

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

Move a helper function to prevent circular dependency #120

Merged

Conversation

HarryMichal
Copy link
Contributor

Since the sanitize_filepart function does not rely on anything in the perun.profile module, it can be safely moved out into a more neutral module.

I encountered the following traceback while playing around with Perun.

Traceback (most recent call last):                                                                                                                                                                                                                            
  File "/var/home/marty/.local/bin/perun", line 5, in <module>                                                                                                                                                                                                
    from perun.cli import launch_cli                                                                                                                                                                                                                          
  File "/var/home/marty/Documents/projects/perun/perun/cli.py", line 55, in <module>                                                                                                                                                                          
    import perun.fuzz.factory as fuzz                                                                                                                                                                                                                         
  File "/var/home/marty/Documents/projects/perun/perun/fuzz/factory.py", line 27, in <module>                                                                                                                                                                 
    import perun.fuzz.evaluate.by_perun as evaluate_workloads_by_perun                                                                                                                                                                                        
  File "/var/home/marty/Documents/projects/perun/perun/fuzz/evaluate/by_perun.py", line 8, in <module>                                                                                                                                                        
    import perun.check.factory as check                                                                                                                                                                                                                       
  File "/var/home/marty/Documents/projects/perun/perun/check/factory.py", line 15, in <module>                                                                                                                                                                
    from perun.profile.helpers import ProfileInfo                                                                                                                                                                                                             
  File "/var/home/marty/Documents/projects/perun/perun/profile/helpers.py", line 29, in <module>                                                                                                                                                              
    import perun.logic.store as store                                                                                                                                                                                                                         
  File "/var/home/marty/Documents/projects/perun/perun/logic/store.py", line 22, in <module>                                                                                                                                                                  
    from perun.profile.factory import Profile                                                                                                                                                                                                                 
  File "/var/home/marty/Documents/projects/perun/perun/profile/factory.py", line 25, in <module>                                                                                                                                                              
    from perun.check.general_detection import get_filtered_best_models_of                                                                                                                                                                                     
  File "/var/home/marty/Documents/projects/perun/perun/check/general_detection.py", line 24, in <module>                                                                                                                                                      
    import perun.check.fast_check as fast_check                                                                                                                                                                                                               
  File "/var/home/marty/Documents/projects/perun/perun/check/fast_check.py", line 13, in <module>                                                                                                                                                             
    import perun.logic.runner as runner                                                                                                                                                                                                                       
  File "/var/home/marty/Documents/projects/perun/perun/logic/runner.py", line 27, in <module>                                                                                                                                                                 
    import perun.collect.trace.optimizations.optimization                                                                                                                                                                                                     
  File "/var/home/marty/Documents/projects/perun/perun/collect/trace/optimizations/optimization.py", line 6, in <module>                                                                                                                                      
    from perun.profile.helpers import sanitize_filepart                                                                                                                                                                                                       
ImportError: cannot import name 'sanitize_filepart' from partially initialized module 'perun.profile.helpers' (most likely due to a circular import) (/var/home/marty/Documents/projects/perun/perun/profile/helpers.py)

@codecov
Copy link

codecov bot commented Oct 6, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (ba11799) 98.04% compared to head (5361b48) 97.95%.

❗ Current head 5361b48 differs from pull request most recent head 9ada459. Consider uploading reports for the commit 9ada459 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##            devel     #120      +/-   ##
==========================================
- Coverage   98.04%   97.95%   -0.10%     
==========================================
  Files         129      129              
  Lines        8686     8686              
==========================================
- Hits         8516     8508       -8     
- Misses        170      178       +8     
Flag Coverage Δ
coverage-3.10 97.91% <100.00%> (-0.09%) ⬇️
coverage-3.11 97.85% <100.00%> (+0.17%) ⬆️
coverage-3.9 97.95% <100.00%> (+0.01%) ⬆️

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

Files Coverage Δ
perun/logic/runner.py 100.00% <100.00%> (ø)
perun/profile/helpers.py 98.77% <100.00%> (-0.02%) ⬇️
perun/utils/helpers.py 100.00% <100.00%> (ø)
perun/view/scatter/run.py 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tfiedor tfiedor requested review from tfiedor and JiriPavela and removed request for tfiedor October 9, 2023 08:23
Copy link
Collaborator

@JiriPavela JiriPavela left a comment

Choose a reason for hiding this comment

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

I fixed the new typing errors that emerged after a new mypy version has been released. I think we're ready to merge now.

@HarryMichal
Copy link
Contributor Author

I think the last two commits can be squashed.

@JiriPavela
Copy link
Collaborator

Good idea, I think we could perhaps go even one step further and do squash&merge of the entire PR. What do you think @tfiedor?

@tfiedor
Copy link
Collaborator

tfiedor commented Oct 24, 2023

Good idea, I think we could perhaps go even one step further and do squash&merge of the entire PR. What do you think @tfiedor?

I am not fan of squash, unless the commiter is dirty bastard (i.e., writing commit messages such as asdf, or blabla, or MYNAME). Which is not this case.

EDIT: Also, you will appreciate more granular history once you bugfix some issue introduced in history, needing git bisect and finding precise chunk which caused error. So I would keep the history as it is, and rather focus on having green checkmarks for the PR.

@HarryMichal
Copy link
Contributor Author

I agree on not squashing the whole PR but the last two commits are related, so I don't see a problem with squashing those two (I regularly amend commits and then force push).

But every maintainer has a different taste, so you decide :).

@tfiedor tfiedor merged commit 7668ca9 into Perfexionists:devel Oct 25, 2023
7 checks passed
@HarryMichal HarryMichal deleted the profile/helpers/circular-deps branch October 26, 2023 12:35
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.

3 participants