-
Notifications
You must be signed in to change notification settings - Fork 17
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 for filter filename not found or not string #7
base: master
Are you sure you want to change the base?
Conversation
general fix to run the example in the readme file
general fix to run the example in readme file
@@ -79,7 +79,7 @@ def __init__(self, | |||
global scipy | |||
|
|||
# check that we were passed a file and it exists | |||
if type(filename) == type(''): | |||
if type(filename) == type(str('')): |
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.
This change doesn't quite make sense to me. What is the value of filename that is causing you trouble? Your proposed change is going to break an important input check.
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.
mmmm in my case if instances were not working because type('') returned "unicode". I just printed type(filename) across all the script and noticed it was always an string instance. So this modification worked for me
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.
This was my problem, even with filters format, directory, and call adress were supposedly correct
ValueError: Please pass the filename for the filter transmission file!
@@ -1025,7 +1025,7 @@ def get_solar_observed_mags(self, | |||
mags = np.zeros((len(zs), len(filters))) | |||
|
|||
# loop through filters one at a time. | |||
for (i, filter) in enumerate(filters): | |||
for (i, filter) in enumerate([filters]): |
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.
filters
should already be a list, so wrapping it in brackets is only going to completely change the meaning of this for loop. What exactly is the problem you are trying to address with this change?
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.
yeah i think this problem was solved with the type(str('')) change. Since if instances were not working, filters reach this line as a string. Since I was too paranoid I just upload all the modifications (i.e. this is not raising an error to me probably because not using that definition or if instance).
Sorry for ambiguity, rookie github user here.
This whole
They are exactly equal, which means that your change doesn't actually change anything. If for some reason there was a weird edge case where the type of
Which is a bit more clear anyway. However, I still don't want to do that for no reason, and I don't understand why your proposed change would actually change anything. |
Yes, they are exactly the same! its probably a problem related to my conda environment, dependencies, etc... |
No description provided.