Alphabetize available_readers method and update documentation #1378
Conversation
There was a problem hiding this comment.
Nice job. I like the additon in the readers.rst file and the sorting looks good to me. Looks like a couple unrelated things were changed that I've commented on inline below. If you could revert those changes or explain why you made them then I'd be happy to merge this.
In case you weren't aware, on github to update a pull request you can just push more commits to the same branch and github will update the PR automatically.
| To get a list of available readers use the `available_readers` function:: | ||
| To get a list of available readers use the `available_readers` function. By default, | ||
| it returns the names of available readers. To return additional reader information | ||
| use `available_readers(as_dict=True)`:: |
| @@ -315,10 +312,13 @@ def available_readers(as_dict=False): | |||
| try: | |||
| reader_info = read_reader_config(reader_configs) | |||
| except (KeyError, IOError, yaml.YAMLError): | |||
| LOG.warning("Could not import reader config from: %s", reader_configs) | |||
There was a problem hiding this comment.
Could you explain why you removed this?
There was a problem hiding this comment.
The person who made the original request recommended that we "suppress [the output] when the reader config is not found as a first time user this creates a lot of clutter."
There was a problem hiding this comment.
Good thinking! Could you just make it a debug instead? That way, regular users won't see it but devs will still get the info.
satpy/readers/__init__.py
Outdated
|
|
||
| Given a mapping where each key is a reader name and each value is a | ||
| """From a mapping from _assign_files_to_readers, get file keys.""" | ||
| """Given a mapping where each key is a reader name and each value is a |
There was a problem hiding this comment.
Not sure what happened with these docstrings, but could you revert these changes (this one and the two above)? I've never seen this formatting for docstrings but I know the docstrings were formatted properly already.
There was a problem hiding this comment.
Sure, the only reason I changed this is because the pre-commit hook kept flagging these lines with "no blank lines allowed after function docstring"
There was a problem hiding this comment.
This means that you are not allowed to have any blank lines between the docstring's last """ and the code iiuc
mraspaud
changed the title
mraspaud
changed the title
Codecov Report
@@ Coverage Diff @@
## master #1378 +/- ##
==========================================
+ Coverage 90.52% 90.56% +0.03%
==========================================
Files 228 228
Lines 33377 33406 +29
==========================================
+ Hits 30215 30254 +39
+ Misses 3162 3152 -10
Continue to review full report at Codecov.
|
|
Looks great so far! We just need to test the new behaviour now. |
|
@alishahusain do you have time to fix a unit test for this? |
Yes, I will send an updated pull request shortly. |
|
Keep in mind you only need to push new commits to this same branch and github will update the pull request. No need to create a new PR. |
|
👉 View analysis in DeepCode’s Dashboard | Configure the bot |
satpy/tests/test_readers.py
Outdated
| @@ -625,12 +625,16 @@ def test_available_readers(self): | |||
| self.assertIsInstance(reader_names[0], str) | |||
| self.assertIn('viirs_sdr', reader_names) # needs h5py | |||
| self.assertIn('abi_l1b', reader_names) # needs netcdf4 | |||
| for i in range(1, len(reader_names)): | |||
There was a problem hiding this comment.
What do you think about self.assertEqual(reader_names, sorted(reader_names))?
|
Closes #1343 |
mraspaud
added
component:readers
enhancement
Alphabetized the list of readers returned by the available_readers() method. Updated documentation so users know they can also do as_dict=True to return more information.
flake8 satpy