-
Notifications
You must be signed in to change notification settings - Fork 27
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
fix conflicting use of datetime.datetime and datetime.timedelta #106
base: main
Are you sure you want to change the base?
Conversation
…time referres to datetime.datetime and therefore timedelta could not be used without import)
Reviewer's Guide by SourceryThis pull request fixes an Updated class diagram for datetime usageclassDiagram
class find_short_baselines {
-dates: np.array
+find_one_year_interferograms(date_list: list[str])
}
note for find_short_baselines.find_one_year_interferograms "Uses datetime.strptime and datetime.timedelta directly after importing them from datetime module."
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
PR summaryThis Pull Request addresses an Review Checklist
SuggestionConsider adding unit tests for the This comment was generated by AI. Information provided may be incorrect. Current plan usage: 0% Have feedback or need help? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @MarritL - I've reviewed your changes - here's some feedback:
Overall Comments:
- This change fixes the immediate error, but consider whether
from datetime import datetime
is the correct import statement throughout the codebase.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Fix:
self.ifgram_dir, self.pairs = self.get_interferogram_pairs()
File "/home/mambauser/tools/MiaplPy/src/miaplpy/miaplpyApp.py", line 436, in get_interferogram_pairs
one_years = fb.find_one_year_interferograms(self.date_list)
File "/opt/conda/lib/python3.10/site-packages/miaplpy/find_short_baselines.py", line 260, in find_one_year_interferograms
dates = np.array([datetime.datetime.strptime(date, '%Y%m%d') for date in date_list])
File "/opt/conda/lib/python3.10/site-packages/miaplpy/find_short_baselines.py", line 260, in
dates = np.array([datetime.datetime.strptime(date, '%Y%m%d') for date in date_list])
AttributeError: type object 'datetime.datetime' has no attribute 'datetime
The file has an import of import datetime.datetime and in function 'find_shortbaseline.py' it assumed an import of only the package datetime, hence the use of datetime.datetime.strptime returned an AttributeError. The same counts for datetime.timedelta (because the imported datetime.datetime has no attribute timedelta).
Summary by Sourcery
Bug Fixes:
datetime.datetime
anddatetime.timedelta
within thefind_short_baselines.py
module. The code was assuming an import of only thedatetime
package, but was referencingdatetime.datetime.strptime
which caused the error.