Mango next #698
Mango next #698
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #698 +/- ##
==========================================
- Coverage 84.09% 84.04% -0.06%
==========================================
Files 80 79 -1
Lines 8556 8535 -21
==========================================
- Hits 7195 7173 -22
- Misses 1361 1362 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
To be continued accordingly to the MANGO recommandation process
to access remote resources
Glossary - improve the test coverage
with local resources only
…) or from a MivotInstance
There was a problem hiding this comment.
Does this file still needed or remained/renamed here as an accident?
There was a problem hiding this comment.
File removed, this was probably due to an unfortunate git add -A
| sparse: boolean, optional (default to False) | ||
| If True, all properties are added in a independent way to the the TEMPLATES. | ||
| They are packed in a MangoObject otherwise. | ||
| schema_check: boolean, optional (default to True) |
There was a problem hiding this comment.
We need to run a docs format checker (to be done separately from this PR), this is just a reminder mostly to myself to do it (these white spaces affect the actual API docs rendering)
| schema_check: boolean, optional (default to True) | |
| schema_check : boolean, optional (default to True) |
There was a problem hiding this comment.
Fixed, but I found out tens of occurrences of this pattern (/[a-z]: /) in many files. I prefer to have your green light before to increase the number of modified files. Could this be done in a specific PR?
There was a problem hiding this comment.
yes, let's not mix those changes into this PR but do it separately, and I'll also try to remember to pick a tool that does that linting for us.
There was a problem hiding this comment.
I'll open a dedicated PR once this one will be merged.
|
|
||
|
|
||
| @prototype_feature('MIVOT') | ||
| class XMLViewer: |
There was a problem hiding this comment.
This was a prototype, so I suppose changing the API without any prior notice is fine and thus this can be removed without deprecation. But this class was mentioned in the narrative docs, so it's just something to be aware of.
There was a problem hiding this comment.
made it more explicit in the change log
pyvo/mivot/viewer/mivot_viewer.py
Outdated
| raise MivotError( | ||
| "Can't find " + Ele.INSTANCE | ||
| + " in " + Ele.TEMPLATES | ||
| ) |
There was a problem hiding this comment.
absolute nitpicking, but I feel this could all fit in one line; we're quite liberal with line lengths and allow them to be up to 110 characters
There was a problem hiding this comment.
A bunch of public methods and attributes have changed here. As the whole module was marked as a prototype I feel it's totally fine, but it is still something we need to keep in mind and maybe explicitly mention in the docs/changelog.
There was a problem hiding this comment.
I made the changelog more explicit while ensuring that it remains compact. I added an attention notice on the top of viewer.rst
2 participants
This MR only relates to the annotation readout.
Code cleaning:
MivotViewer:
This PR improves the current API. It will be extended with model specific modules once MANGO will be a REC. At that time new MANGO-specific features will be completed.